-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-134918: Fix and improve doctest's documentation #134919
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
bdcf54a
to
1c9791a
Compare
@tim-one, please look at the floating point examples. |
@@ -1940,7 +1939,7 @@ several options for organizing tests: | |||
containing test cases for the named topics. These functions can be included in | |||
the same file as the module, or separated out into a separate test file. | |||
|
|||
* Define a ``__test__`` dictionary mapping from regression test topics to | |||
* Define a :attr:`~module.__test__` dictionary mapping from regression test topics to |
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.
Should this be a Sphinx cross-reference? Seems like it shouldn't; I wouldn't think module.__test__
is a valid cross-ref target within 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.
It refers to the attribute module.__test__
that was added in this PR as well: https://github.com/python/cpython/pull/134919/files#diff-035fbece7e3400413ea9a79196a2e96e0d6c9b5ed92ab8f5285c0600f7d325caR314.
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.
Ah, interesting, I see how that's set up now, thanks.
Isn't this an "off-label" (no pun intended) use of attribute
, though? I would expect this to be implemented using a label. It seems odd that an attribute
would link to something other than the API docs for (in this case) what would be module.__test__
.
I see the benefit here, in that it results in __test__
being monospace-formatted. But it seems like a semantic mismatch to me.
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.
We can say that this attribute may or may not exist, and dunder names are technically reserved for the language so I'm ok with this.
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.
Makes sense, I'm good with it.
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.
Yet one benefit is adding "__test__ (module attribute)
" in the index.
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
…4919) (cherry picked from commit 3c66e59) Co-authored-by: Serhiy Storchaka <[email protected]>
…4919) (cherry picked from commit 3c66e59) Co-authored-by: Serhiy Storchaka <[email protected]>
GH-134966 is a backport of this pull request to the 3.14 branch. |
GH-134967 is a backport of this pull request to the 3.13 branch. |
…GH-134966) (cherry picked from commit 3c66e59) Co-authored-by: Serhiy Storchaka <[email protected]>
…GH-134967) (cherry picked from commit 3c66e59) Co-authored-by: Serhiy Storchaka <[email protected]>
📚 Documentation preview 📚: https://cpython-previews--134919.org.readthedocs.build/