Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-88427: Deprecatesocket.SocketType#126272

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

Draft
rruuaanng wants to merge48 commits intopython:main
base:main
Choose a base branch
Loading
fromrruuaanng:gh88427

Conversation

rruuaanng
Copy link
Contributor

@rruuaanngrruuaanng commentedNov 1, 2024•
edited
Loading

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

@rruuaanngrruuaanng changed the titlegh88427: Deprecatedsocket.SocketTypegh-88427: Deprecatedsocket.SocketTypeNov 1, 2024
Copy link
Member

@ZeroIntensityZeroIntensity left a 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 aDeprecationWarning when accessing aSocketType (you can do this via aPEP-562__getattr__), and then add the documentation notes.

@rruuaanng
Copy link
ContributorAuthor

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

@JelleZijlstra
Copy link
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 andsocket modules that raises a DeprecationWarning and then returns_socket.socket.

@rruuaanng
Copy link
ContributorAuthor

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

@rruuaanng
Copy link
ContributorAuthor

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

@picnixz
Copy link
Member

It seems that the modulegetattr 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, readPEP-562 for examples on how to use it.

@rruuaanng
Copy link
ContributorAuthor

rruuaanng commentedNov 2, 2024•
edited
Loading

It seems that the modulegetattr 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, readPEP-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.

@ZeroIntensity
Copy link
Member

I think you need to raise anAttributeError from the__getattr__ if it isn'tSocketType.

rruuaanng reacted with thumbs up emoji

@rruuaanngrruuaanng marked this pull request as ready for reviewNovember 2, 2024 01:50
Copy link
Member

@vstinnervstinner left a 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.

@rruuaanng
Copy link
ContributorAuthor

@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'>

@rruuaanngrruuaanng marked this pull request as draftNovember 19, 2024 13:53
@vstinner
Copy link
Member

I forgot the most important step

Right. Please test your change before publishing it.

rruuaanng reacted with laugh emoji

@rruuaanngrruuaanng marked this pull request as ready for reviewDecember 8, 2024 12:37
@rruuaanng
Copy link
ContributorAuthor

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

@vstinner
Copy link
Member

test_socket and test_pickle are failing on the CI Ubuntu.

@rruuaanng
Copy link
ContributorAuthor

rruuaanng commentedDec 10, 2024•
edited
Loading

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.pyRunning 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.918sOK (skipped=485)

test_pickle.py

PS E:\code\github\cpython\alpha\cpython-main> ./python -m unittest Lib\test\test_pickle.pyRunning 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.836sOK (skipped=30)

I tried removing my changes and rantest_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.pyRunning 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.947sOK (skipped=30)

test_socket.py passes the test perfectly fine and throws the deprecation, buttest_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.

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;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Shouldsock_type getINCREF'd? I'm assuming its a static type and therefore immortal, but we should add a comment clarifying that.

Copy link
ContributorAuthor

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 wheresock_type is used insocketmodule.c. In the scenario wherePy_DECREF is used, onlysocket_clear usesPy_DECREF. Andsock_type does not usePy_INCREF. And in other parts, it seems that there is no special comment.

@rruuaanngrruuaanng marked this pull request as draftDecember 12, 2024 11:41
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@JelleZijlstraJelleZijlstraJelleZijlstra left review comments

@picnixzpicnixzpicnixz left review comments

@ZeroIntensityZeroIntensityZeroIntensity requested changes

@Eclips4Eclips4Eclips4 left review comments

@vstinnervstinnerAwaiting requested review from vstinner

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

7 participants
@rruuaanng@JelleZijlstra@picnixz@ZeroIntensity@Wulian233@vstinner@Eclips4
[8]ページ先頭
©2009-2025 Movatter.jp