Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
GH-101100: Fix reference warnings forsocket
methods#110114
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
I think that in all cases where it refers to socket() as a function, it should keep :func: role.
Doc/library/socket.rst Outdated
@@ -23,7 +23,7 @@ all modern Unix systems, Windows, MacOS, and probably additional platforms. | |||
The Python interface is a straightforward transliteration of the Unix system | |||
call and library interface for sockets to Python's object-oriented style: the | |||
:func:`.socket` function returns a :dfn:`socket object` whose methods implement | |||
:class:`.socket` function returns a :dfn:`socket object` whose methods implement |
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 says "the socket() function". I think that it is better to leave references to class as a function in the context where the fact that it is a callable is more important than the fact that it is also a type. There are a lot of precedences withint()
,str()
,range()
,map()
, etc.
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.
Would you suggest adding a.. function:: socket
directive (potentially just above.. class:: socket
)?
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 is not needed, the role type should not affect links. For example:func:`int`
and:class:`int`
refer to the same.. class::
declaration. The difference is that :func: adds ().
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.
I tried this locally (replacing all:class:`.socket`
with:func:`socket`
) -- the problem is that the cross-reference goes to the module object definition (library/socket.html#module-socket) rather than the socket callable.
So I think our best option is to add a.. function:: socket
directive.
A
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.
Looking further,socket()
was previously documented as a function, but#23960 changed it to be documented as a class. Perhaps we could change it back?map
is documented as a function, for example.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
I think that the problem is that if you use a reference starting with.
, Sphinx switches in more strict mode that checks the role type, so:func:`.socket`
does not find theclass
declaration. But:func:`socket.socket`
may work.
@AA-Turner Please could you resolve the conflicts? Then I think we're ready to merge. |
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.
I restored the use of the.. class::
directive forsocket
. It fixes rendering for "the socket constructor". The:func:
references work when specify the full qualified name.
Also fixed references for few more methods.
Thanks@AA-Turner for the PR, and@hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
…nGH-110114)(cherry picked from commitffe1b2d)Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…nGH-110114)(cherry picked from commitffe1b2d)Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
GH-112455 is a backport of this pull request to the3.12 branch. |
GH-112456 is a backport of this pull request to the3.11 branch. |
…n#110114)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
…n#110114)Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Uh oh!
There was an error while loading.Please reload this page.
This doesn't tackle the constants.
📚 Documentation preview 📚:https://cpython-previews--110114.org.readthedocs.build/en/110114/library/socket.html