Skip to content

Conversation

@AA-Turner
Copy link
Member

@AA-TurnerAA-Turner commented Sep 29, 2023

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a 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.

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

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 with int(), str(), range(), map(), etc.

Copy link
MemberAuthor

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)?

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 ().

Copy link
MemberAuthor

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

Copy link
MemberAuthor

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.

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a 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 the class declaration. But :func:`socket.socket` may work.

@hugovk
Copy link
Member

@AA-Turner Please could you resolve the conflicts? Then I think we're ready to merge.

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a 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 for socket. It fixes rendering for "the socket constructor". The :func: references work when specify the full qualified name.

Also fixed references for few more methods.

@hugovkhugovk merged commit ffe1b2d into python:mainNov 27, 2023
@miss-islington-app
Copy link

Thanks @AA-Turner for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 27, 2023
…nGH-110114) (cherry picked from commit ffe1b2d) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 27, 2023
…nGH-110114) (cherry picked from commit ffe1b2d) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app
Copy link

GH-112455 is a backport of this pull request to the 3.12 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.12 only security fixes label Nov 27, 2023
@bedevere-app
Copy link

GH-112456 is a backport of this pull request to the 3.11 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.11 only security fixes label Nov 27, 2023
hugovk pushed a commit that referenced this pull request Nov 27, 2023
…10114) (#112456) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
hugovk pushed a commit that referenced this pull request Nov 27, 2023
…10114) (#112455) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
…n#110114) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…n#110114) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docsDocumentation in the Doc dirskip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@AA-Turner@hugovk@serhiy-storchaka@ezio-melotti