Skip to content

docs: update API page, add type annotations #4424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 5, 2022

Conversation

bastimeyer
Copy link
Member

This updates the API page in the docs, which has been a total mess since years (or basically since forever).

Changes

Docs / Sphinx:

Streamlink:

  • Update all class/method/attribute docstrings that get parsed by autodoc
    • Fix typos and spelling/grammar errors (I didn't read everything, but checked most)
    • This isn't a full rewrite of the existing docstrings, just a cleanup, so there's still lots of room for improvements.
  • Add a couple of new docstrings for relevant methods and attributes
  • Where possible, add type annotations instead of defining parameter and return types via docstrings.

Issues

Autodoc:

  • Autodoc renders certain parameters incorrectly, eg. session instead of session_ (HTTPStream / HLSStream). These method signatures need to be updated in the future because the underscore doesn't make any sense. Fixing that would be a breaking change, so I suggest waiting for the py37 removal (EOL June 2023) when positional-only and keyword-only parameters become available, so this can be done in one go.
  • The styling of the autodoc output is not particularly great and it gets really messy when methods have large signatures with type annotations. This can only be worked around with ugly CSS hacks, which isn't worth it.

Streamlink:

  • Most types can't be annotated because of import cycle issues. Example of this are streamlink.stream.Stream which can't be imported in streamlink.plugin.plugin, or streamlink.session.Streamlink (in various places). That unfortunately requires a redesign. Annotating Plugin.session is also currently impossible because of the Plugin.bind classmethod which I've commented on a few weeks ago.

Next

  1. Improve the streaming protocol parameters documentation, because the parameters get passed to the various stream implementations and requests.request(), and my intention was to link to the classes/methods.

Future improvements

  1. Add intersphinx config for the standard library, requests and other dependencies, so that the docs can link to the documentations of the classes and methods which are mentioned in Streamlink's docstrings.
    https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html

@bastimeyer
Copy link
Member Author

Copy link
Member

@gravyboat gravyboat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good. Big 👍 for fixing the grammar issues and lack of clarity on some of the entries. I'll mark as approved instead of merging right away in case anyone else wants to review just in case I missed something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants