GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-88427: Deprecate `socket.SocketType` by rruuaanng · Pull Request #126272 · python/cpython

Conversation

@rruuaanng

@rruuaanng rruuaanng commented Nov 1, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

I propose to deprecate socket.SocketType. There's no reason to publicly expose _socket.socket separately from socket.socket, the only type that moat users should care about.
SocketType isn't needed for type checking; socket.socket is itself a class and can be used in type annotations.

cc @JelleZijlstra

  • Issue: SocketType documentation misleading #88427

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@rruuaanng rruuaanng changed the title gh88427: Deprecated socket.SocketType gh-88427: Deprecated socket.SocketType Nov 1, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Nov 1, 2024
SocketType documentation misleading #88427
Open
ZeroIntensity
ZeroIntensity requested changes Nov 1, 2024
View reviewed changes

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

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.

SocketType isn't deprecated right now, so we can't remove it yet.

Please update the PR to raise a DeprecationWarning when accessing a SocketType (you can do this via a PEP-562 __getattr__), and then add the documentation notes.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added the awaiting core review label Nov 1, 2024
@rruuaanng

rruuaanng commented Nov 1, 2024

Copy link
Copy Markdown
Contributor Author

Maybe we can wait for the promoter to answer. I won't open this PR before I decide whether to abandon or remove it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@JelleZijlstra

JelleZijlstra commented Nov 1, 2024

Copy link
Copy Markdown
Member

I looked at the code again and agree with my opinion from 2021: it should be deprecated and eventually removed.

It could be useful to do a code search (e.g., GitHub code search, Hugo's top 5k pypi packages tool) to see how the name is being used.

A possible approach to deprecation could be to remove the current line adding it to _socket, and add a module __getattr__ to both the _socket and socket modules that raises a DeprecationWarning and then returns _socket.socket.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rruuaanng

rruuaanng commented Nov 1, 2024

Copy link
Copy Markdown
Contributor Author

Okay, I'll re-revise this PR so that it throws a warning when calling this type. Call me back in this PR when we consider removing it in the future :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rruuaanng
deprecated SocketType
341a29e
@rruuaanng
rruuaanng force-pushed the gh88427 branch from e68ac9b to 341a29e Compare November 1, 2024 14:51
rruuaanng added 2 commits November 1, 2024 23:09
@rruuaanng
_socket.socket change to _socket.SocketType
a4ef847
@rruuaanng
Remove return
5a57cb1
ZeroIntensity
ZeroIntensity reviewed Nov 1, 2024
View reviewed changes
Comment thread Doc/library/socket.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/socket.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Library/2024-11-01-20-09-53.gh-issue-88427.chYNT6.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
rruuaanng added 2 commits November 1, 2024 23:19
@rruuaanng
Change docs
72f7c3d
@rruuaanng
Change NEWS
58b305d
picnixz
picnixz reviewed Nov 1, 2024
View reviewed changes
Comment thread Lib/socket.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/socket.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Library/2024-11-01-20-09-53.gh-issue-88427.chYNT6.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
rruuaanng added 3 commits November 1, 2024 23:25
@rruuaanng
Change docs
73b14ea
@rruuaanng
Change NEWS
2c2e8e3
@rruuaanng
Move import
3ea9169
picnixz
picnixz reviewed Nov 1, 2024
View reviewed changes
Comment thread Lib/socket.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
rruuaanng added 2 commits November 1, 2024 23:45
@rruuaanng
Add return
3b7d187
@rruuaanng
socket rename to SocketType
188a924
@rruuaanng

rruuaanng commented Nov 2, 2024

Copy link
Copy Markdown
Contributor Author

It seems that the module __getattr__ only works for attributes that do not exist, which makes it necessary to remove the export in socketmodule. But it triggers a deprecation warning. Emmm...
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
rruuaanng added 2 commits November 2, 2024 08:52
@rruuaanng
Remove defined
ce4bfa6
@rruuaanng
Return _socket.socket
362dbab
@picnixz

picnixz commented Nov 2, 2024

Copy link
Copy Markdown
Member

It seems that the module getattr only works for attributes that do not exist, which makes it necessary to remove the export in socketmodule. But it triggers a deprecation warning. Emmm...

It works for any kind of attributes. Please, read PEP-562 for examples on how to use it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rruuaanng

rruuaanng commented Nov 2, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

It seems that the module getattr only works for attributes that do not exist, which makes it necessary to remove the export in socketmodule. But it triggers a deprecation warning. Emmm...

It works for any kind of attributes. Please, read PEP-562 for examples on how to use it.

I read the document and modified it. But why does it trigger this

Traceback (most recent call last):
  File "D:\a\cpython\cpython\Lib\test\pythoninfo.py", line 1062, in collect_info
    collect_func(info_add)
    ~~~~~~~~~~~~^^^^^^^^^^
  File "D:\a\cpython\cpython\Lib\test\pythoninfo.py", line 727, in collect_test_socket
    from test import test_socket
  File "D:\a\cpython\cpython\Lib\test\test_socket.py", line 201, in <module>
    HAVE_SOCKET_CAN = _have_socket_can()
  File "D:\a\cpython\cpython\Lib\test\test_socket.py", line 104, in _have_socket_can
    s = socket.socket(socket.PF_CAN, socket.SOCK_RAW, socket.CAN_RAW)
  File "D:\a\cpython\cpython\Lib\socket.py", line 242, in __init__
    _socket.socket.__init__(self, family, type, proto, fileno)
    ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'NoneType' object cannot be interpreted as an integer

Edit
It seems to be a consistent bug since I opened the PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ZeroIntensity

ZeroIntensity commented Nov 2, 2024

Copy link
Copy Markdown
Member

I think you need to raise an AttributeError from the __getattr__ if it isn't SocketType.
rruuaanng reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rruuaanng
Add AttributeError
5042f2f
@rruuaanng
rruuaanng marked this pull request as ready for review November 2, 2024 01:50
@bedevere-app bedevere-app Bot removed the awaiting core review label Nov 2, 2024
vstinner
vstinner reviewed Nov 19, 2024
View reviewed changes
Comment thread Modules/socketmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner reviewed Nov 19, 2024
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

@rruuaanng: Did you try your change before updating the PR? It doesn't work, no warning is emitted:

$ ./python
>>> import _socket
>>> _socket.SocketType
<class '_socket.socket'>

I suggest to mark the PR as a draft since it changes many times in a short time.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@rruuaanng

rruuaanng commented Nov 19, 2024

Copy link
Copy Markdown
Contributor Author

@vstinner
I forgot the most important step, removing its definition, and now it can successfully trigger deprecation ;-)

>>> import _socket
>>> _socket.SocketType
<python-input-1>:1: DeprecationWarning: It will be removed in 3.16. Use socket.socket type instead
  _socket.SocketType
<class '_socket.socket'>
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rruuaanng
Remove SocketType original definition
4d71bce
@rruuaanng
rruuaanng marked this pull request as draft November 19, 2024 13:53
@bedevere-app bedevere-app Bot removed the awaiting merge label Nov 19, 2024
@vstinner

vstinner commented Nov 19, 2024

Copy link
Copy Markdown
Member

I forgot the most important step

Right. Please test your change before publishing it.
rruuaanng reacted with laugh emoji
All reactions
  • 😄 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner reviewed Nov 19, 2024
View reviewed changes
Comment thread Lib/socket.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
ZeroIntensity
ZeroIntensity reviewed Nov 19, 2024
View reviewed changes
Comment thread Modules/socketmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
rruuaanng and others added 6 commits November 19, 2024 22:09
@rruuaanng
_socket.socket change to _socket.SocketType
347c8e3
@rruuaanng
strcmp change to PyUnicode_EqualToUTF8
e625c64
@rruuaanng
Merge branch 'gh88427' of https://github.com/rruuaanng/cpython into g…
a909333 
…h88427
@rruuaanng
Use a better implementation
0d836c4
@rruuaanng
Clear CI error
38efc89
@rruuaanng
Add cast
e07a953
@rruuaanng
rruuaanng marked this pull request as ready for review December 8, 2024 12:37
@bedevere-app bedevere-app Bot added the awaiting review label Dec 8, 2024
@rruuaanng
rruuaanng requested review from ZeroIntensity and vstinner December 8, 2024 12:37
@rruuaanng
use _socket.SocketType
fae620a
@rruuaanng

rruuaanng commented Dec 9, 2024

Copy link
Copy Markdown
Contributor Author

Please allow me to check again to see if there are any unresolved comments to decide whether we should remove the DO_NOT_MERGE tag.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rruuaanng
Merge branch 'main' into gh88427
f029878
@vstinner

vstinner commented Dec 9, 2024

Copy link
Copy Markdown
Member

test_socket and test_pickle are failing on the CI Ubuntu.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rruuaanng

rruuaanng commented Dec 10, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Oh.... I don't understand, I was looking for what was throwing the error so I ran the failing tests locally and it seemed like they all worked fine on my computer.

test_socket.py

PS E:\code\github\cpython\alpha\cpython-main> ./python -m unittest Lib\test\test_socket.py
Running Release|x64 interpreter...
.ss.sssssssss.......sssssssss.......................sss...ssss........ssssss.....................s...........................E:\code\github\cpython\alpha\cpython-main\Lib\socket.py:74: DeprecationWarning: _socket.SocketType is deprecated and will be removed in Python 3.16. Use socket.socket instead
  return _socket.SocketType
.s........s...s............sss.......s..........ssssss..s.s.ssssssssssss..........sssssss...............s..ss.....sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss...........ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss...........s..ss..ssssss.s.....sssssssss...............................................
----------------------------------------------------------------------
Ran 732 tests in 69.918s

OK (skipped=485)

test_pickle.py

PS E:\code\github\cpython\alpha\cpython-main> ./python -m unittest Lib\test\test_pickle.py
Running Release|x64 interpreter...
...........s.................................................................................................................................s...........................................................................................................................................................s........................sss..............sss.......Can't import module 'multiprocessing.popen_fork': module 'os' has no attribute 'WNOHANG'
No module named '_gdbm'
No module named '_gdbm'
No module named 'dbm.bsd'
No module named '_dummy_thread'
No module named '_dbm'
No module named '_dbm'
.No module named '_dummy_thread'
No module named 'dbm.bsd'
No module named '_dbm'
No module named '_gdbm'
No module named '_dbm'
No module named '_gdbm'
..Can't import module 'multiprocessing.popen_fork': module 'os' has no attribute 'WNOHANG'
E:\code\github\cpython\alpha\cpython-main\Lib\socket.py:74: DeprecationWarning: _socket.SocketType is deprecated and will be removed in Python 3.16. Use socket.socket instead
  return _socket.SocketType
.No module named '_dummy_thread'
No module named '_dummy_thread'
No module named 'dbm.bsd'
No module named 'dbm.bsd'
No module named '_dbm'
No module named '_dbm'
No module named '_gdbm'
No module named '_gdbm'
.Can't import module 'multiprocessing.popen_fork': module 'os' has no attribute 'WNOHANG'
E:\code\github\cpython\alpha\cpython-main\Lib\socket.py:74: DeprecationWarning: _socket.SocketType is deprecated and will be removed in Python 3.16. Use socket.socket instead
  return _socket.SocketType
....s..........................................................................................................................s...............................sssss....sss......................sss...........................................................................................................s............................................................................................................................s......................................................sss..............sss........
----------------------------------------------------------------------
Ran 865 tests in 13.836s

OK (skipped=30)

I tried removing my changes and ran test_pickle.py again, but the only difference was that the deprecation was gone.

PS E:\code\github\cpython\alpha\cpython-main> ./python -m unittest Lib\test\test_pickle.py
Running Release|x64 interpreter...
...........s.................................................................................................................................s...........................................................................................................................................................s........................sss..............sss.......No module named '_gdbm'
Can't import module 'multiprocessing.popen_fork': module 'os' has no attribute 'WNOHANG'
No module named '_dbm'
No module named 'dbm.bsd'
No module named '_dummy_thread'
No module named '_gdbm'
No module named '_dbm'
.No module named '_dummy_thread'
No module named 'dbm.bsd'
No module named '_dbm'
No module named '_gdbm'
No module named '_dbm'
No module named '_gdbm'
..Can't import module 'multiprocessing.popen_fork': module 'os' has no attribute 'WNOHANG'
.No module named '_dummy_thread'
No module named '_dummy_thread'
No module named 'dbm.bsd'
No module named 'dbm.bsd'
No module named '_dbm'
No module named '_dbm'
No module named '_gdbm'
No module named '_gdbm'
.Can't import module 'multiprocessing.popen_fork': module 'os' has no attribute 'WNOHANG'
....s..........................................................................................................................s...............................sssss....sss......................sss...........................................................................................................s............................................................................................................................s......................................................sss..............sss........
----------------------------------------------------------------------
Ran 865 tests in 15.947s

OK (skipped=30)

test_socket.py passes the test perfectly fine and throws the deprecation, but test_pickle.py seems to throw an error that seems unrelated to the PR. But it is undeniable that my CI did not pass :(

Edit

test_pickle.py threw the same exception as my local test results on all three platforms.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ZeroIntensity
ZeroIntensity reviewed Dec 10, 2024
View reviewed changes
Comment thread Modules/socketmodule.c
PyErr_Warn(PyExc_DeprecationWarning, "_socket.SocketType is deprecated and "
"will be removed in Python 3.16. "
"Use socket.socket instead");
return state != NULL ? (PyObject *)state->sock_type : NULL;

@ZeroIntensity ZeroIntensity Dec 10, 2024

Copy link
Copy Markdown
Member

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 sock_type get INCREF'd? I'm assuming its a static type and therefore immortal, but we should add a comment clarifying that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@rruuaanng rruuaanng Dec 10, 2024

Copy link
Copy Markdown
Contributor Author

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 to search for situations where sock_type is used in socketmodule.c. In the scenario where Py_DECREF is used, only socket_clear uses Py_DECREF. And sock_type does not use Py_INCREF. And in other parts, it seems that there is no special comment.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@rruuaanng
rruuaanng marked this pull request as draft December 12, 2024 11:41
@bedevere-app bedevere-app Bot removed the awaiting review label Dec 12, 2024
@github-actions

github-actions Bot commented Apr 19, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 19, 2026
@gaborbernat gaborbernat mentioned this pull request May 31, 2026
gh-89554: Document socket.SocketType as a class #150683
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments
@picnixz picnixz picnixz left review comments
@ZeroIntensity ZeroIntensity ZeroIntensity requested changes
@Eclips4 Eclips4 Eclips4 left review comments
@vstinner vstinner Awaiting requested review from vstinner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

DO-NOT-MERGE stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@rruuaanng @JelleZijlstra @picnixz @ZeroIntensity @Wulian233 @vstinner @Eclips4
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.