gh-151950: Fix Sphinx reference warnings in wsgiref docs#154498
Conversation
Documentation build overview
11 files changed ·
|
|
The purpose was to document those attributes, not suppress them. Please do so. |
picnixz
left a comment
There was a problem hiding this comment.
Please document the header objects properly.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase And if you don't make the requested changes, you will be put in the comfy chair! |
Replace the "!"-suppressed references with real targets: .. method:: directives for Headers.keys(), Headers.values() and Headers.items(), and .. attribute:: directives for WSGIServer.base_environ, BaseHandler.environ and SimpleHandler's stdin, stdout and stderr. Correct the SimpleHandler paragraph while documenting it: the constructor stores the supplied environment in base_env, not environ, and add_cgi_vars() merges it into BaseHandler.environ during setup_environ(). paste.lint (a third-party module) and FileWrapper.close (bound conditionally from the wrapped object, never defined on the class) keep the "!" prefix, since neither can have a real target.
|
Thanks, that's fair. I've replaced the suppressions with real targets:
One thing I ran into while doing that: the Two references are still suppressed, and I believe that is correct in both cases:
One more that I left alone deliberately, so tell me if you want it in scope: the class description still points I have made the requested changes; please review again |
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
picnixz
left a comment
There was a problem hiding this comment.
If possible add versionadded/versionchanged directives to methods/atters that were not present at the beginning (i.e. if they were added later).
And also, if you using an agent, make sure that you review its output. Agents are quite bad at placement in my experience.
| ``len()`` of a :class:`Headers` object is the same as the length of its | ||
| :meth:`items`, which is the same as the length of the wrapped header list. In | ||
| fact, the :meth:`items` method just returns a copy of the wrapped header list. | ||
| :meth:`items`, which is the same as the length of the wrapped header list. |
| :meth:`items`, which is the same as the length of the wrapped header list. | ||
|
|
||
|
|
||
| .. method:: Headers.keys() |
There was a problem hiding this comment.
Above we say that Headers implement dict.get. So we could maybe reference dict.keys() instead. I do not remember whether it is implemented specifically or not. If .keys() etc are explicit methods on the Header class, you can keep this.
| :meth:`get_app` exists mainly for the benefit of request handler instances. | ||
|
|
||
|
|
||
| .. attribute:: WSGIServer.base_environ |
There was a problem hiding this comment.
Check if this page documents attributes before methods or vice-versa first. I believe attributes are put first. Then move that one accordingly. It is weird to have this attribute documentation here
| and the :attr:`server_software` attribute is set. | ||
|
|
||
|
|
||
| .. attribute:: BaseHandler.environ |
There was a problem hiding this comment.
Move this in the attributes section
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Fixes the 18 nit-picky Sphinx reference warnings in
Doc/library/wsgiref.rstand removes the file fromDoc/tools/.nitignore.Where a correct target already exists, the reference is qualified so it resolves:
readandclosebecome:meth:`~io.BufferedIOBase.read``` and:meth:~io.IOBase.close```. This matches the existing ``:meth:~io.BufferedIOBase.write``` reference further down the same file.shift_path_infoandguess_schemeare documented inwsgiref.utilbut were referenced from a different module context, so they are now qualified.serve_foreverandhandle_requestpoint atsocketserver.BaseServer, which is whereWSGIServeractually inherits them from.The remaining names have no documented target anywhere:
Headers.keys,Headers.values,Headers.items,WSGIServer.base_environ,BaseHandler.environandSimpleHandler.stdin/stdout/stderrare described only in prose, andpaste.lintis third party. Those use the!prefix, which keeps the semantic markup and drops the link.I kept this to reference fixes so the diff stays limited to removing the warnings. If you would prefer the undocumented
Headersmethods and handler attributes to become proper.. method::and.. attribute::entries instead (as was done for the lzma constants in gh-151949), I am happy to do that here or in a follow-up.Verified with a fresh nit-picky build: warnings for this file go from 18 to 0, total build warnings go from 1352 to 1334, no new warnings elsewhere, and
Doc/tools/check-warnings.py --fail-if-regression --fail-if-improvedexits 0.Doc/library/files #151950