-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
docs: add plugin.api.validate API documentation #5652
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
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
cc6a46e to
79960ef
Compare
79960ef to
f9f39e8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a bit but I looked at everything. I don't see any typos or grammar issues. I've noted one small point about the doc formatting for builtins but it's not a show stopper nor does it need to be considered here. Looks really good!
| Please note the difference between :class:`list <builtins.list>` | ||
| and the :class:`ListSchema <_schemas.ListSchema>` validation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential slight chance of confusion here as people may think you are referring to streamlink.plugin.api.validate.list which itself is an alias of ListSchema if they aren't familiar with the doc formatting. Not a big deal but it's easier to understand the difference for something like https://github.com/streamlink/streamlink/pull/5652/files#diff-010b6199b0fc1ebe5df5bdb5ee86c9d024fea4dca02b63f388fe11b1d5156599R216 because of the reference to re.pattern.
I wish there was a way <builtins.list> could easily link to the python docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
people may think you are referring to
streamlink.plugin.api.validate.list
Maybe, but if they check validate.list, they'll see that it's an alias for ListSchema, which means that this line compares the same things, which doesn't make much sense and it must be two different things. All members of the validate module are always referenced with their module name, never on their own, apart from schema class names.
I wish there was a way
<builtins.list>could easily link to the python docs.
It's currently disabled. For a good reason, because it's way too noisy with external links everywhere...
https://github.com/streamlink/streamlink/blob/6.3.1/docs/conf.py#L108-L111
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me.
|
I'll fix the order of items later today or tomorrow and then merge. Any follow-up issues can be fixed afterwards. |
f9f39e8 to
d7e8ec3
Compare
|
Just to document this: The reason why autodoc doesn't fully document the schema classes in the main module and instead outputs "alias of" is that the classes' This could be fixed by overriding the def _(items):
for name, obj in items:
if type(obj) is type and obj.__module__.startswith(f"{__name__}."):
obj.__name__ = name
_(globals().items())
del _But this would change the validation error message and it would require fixing the tests. I'll have a look at this in the future, because it also requires some changes in the docstrings in regards to the type annotations (the list issue mentioned above now being an actual issue). |
This finally adds the API documentation for
plugin.api.validate. An API guide with real-life examples is not included yet and will be added later.This was much more work than expected. Especially because of several sphinx-autodoc issues which required me to reformat and move everything multiple times.
I've tried to make the style as consistent as possible, but there are some inconsistencies between the
validatedocs, schema docs and utility function docs. These are just minor differences though and you won't notice them. I won't even tell you.As explained in the top comment of
validate.rst, the layout was chosen deliberately because of certain autodoc quirks. Unfortunately, it's not ideal, but it's the best I could come up with.My first attempt was documenting all overloading function of
validate()(base schema types, as well as custom schemas), but then I noticed that autodoc doesn't supportsingledispatchproperly.Manually importing the functions using
autofunctionalso was a bad idea, because this would require duplicate docs, on both the schema validation functions and the schema classes themselves.So I ended up with the current appoach of adding custom function docs in the
validate.rstfile for the basic schema validation functions liketype,abc.Callable,list(etc),dictandre.Pattern, and importing the rest with proper docstrings in the python modules.