Uh oh!
There was an error while loading. Please reload this page.
gh-88427: Deprecate socket.SocketType - #126272
Conversation
socket.SocketTypesocket.SocketType
ZeroIntensity
left a comment
There was a problem hiding this comment.
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.
rruuaanng
commented
Nov 1, 2024
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
commented
Nov 1, 2024
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 |
rruuaanng
commented
Nov 1, 2024
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 :) |
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.
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.
Uh oh!
There was an error while loading. Please reload this page.
rruuaanng
commented
Nov 2, 2024
It seems that the module |
picnixz
commented
Nov 2, 2024
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 Edit |
ZeroIntensity
commented
Nov 2, 2024
I think you need to raise an |
Uh oh!
There was an error while loading. Please reload this page.
vstinner
left a comment
There was a problem hiding this comment.
@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
commented
Nov 19, 2024
@vstinner |
vstinner
commented
Nov 19, 2024
Right. Please test your change before publishing it. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
rruuaanng
commented
Dec 9, 2024
Please allow me to check again to see if there are any unresolved comments to decide whether we should remove the |
vstinner
commented
Dec 9, 2024
test_socket and test_pickle are failing on the CI Ubuntu. |
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 test_pickle.py I tried removing my changes and ran
Edit
|
| 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; |
There was a problem hiding this comment.
Should sock_type get INCREF'd? I'm assuming its a static type and therefore immortal, but we should add a comment clarifying that.
There was a problem hiding this comment.
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.
This PR is stale because it has been open for 30 days with no activity. |
cc @JelleZijlstra