FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-105373: Remove the PyObject_SetAttr(NULL) deprecation by vstinner · Pull Request #105374 · python/cpython · GitHub

Repository navigation

gh-105373: Remove the PyObject_SetAttr(NULL) deprecation - #105374

Closed
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:keep_obj_setattr_null
Closed

vstinner wants to merge 1 commit into
python:mainfrom
vstinner:keep_obj_setattr_null

Conversation

vstinner commented Jun 6, 2023 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

Remove the deprecation on the following functions:

  • PyObject_SetAttr(obj, attr_name, NULL)
  • PyObject_SetAttrString(obj, attr_name, NULL)
  • PySequence_SetItem(obj, i, NULL)
  • PySequence_SetSlice(obj, i1, i2, NULL)

Deprecation added by commit ed82604.


📚 Documentation preview 📚: https://cpython-previews--105374.org.readthedocs.build/

Remove the deprecation on the following functions:

* PyObject_SetAttr(obj, attr_name, NULL)
* PyObject_SetAttrString(obj, attr_name, NULL)
* PySequence_SetItem(obj, i, NULL)
* PySequence_SetSlice(obj, i1, i2, NULL)

Deprecation added by commit ed82604.

vstinner commented Jun 6, 2023

Copy link
Copy Markdown
Member Author

Deprecation added by issue #69887. cc @serhiy-storchaka @vadmium

I would prefer that either we fully removed these deprecations, or to really deprecate them: emit DeprecationWarning at runtime.

Copy link
Copy Markdown
Member

What is changed since it was discussed 8 years ago?

vstinner commented Jun 6, 2023

Copy link
Copy Markdown
Member Author

What is changed since it was discussed 8 years ago?

I would like to clarify what is really deprecated or not in Python 3.13. I don't see the value of a mention of a deprecation in the documentation without using .. deprecated:: markup and without emitting a DeprecationWarning at runtime.

IMO passing NULL is just fine, it's not going to change, so there is no need to deprecate it.

In 2015, you wrote:

I think the conclusion of Python_Dev discussion is that it is not worth to deprecate this behavior in the code.

So well, I propose to actually not deprecate it :-D

Copy link
Copy Markdown
Member

The Gmane link no longer work, the working links to the discussion (it was split on several threads) are
https://mail.python.org/archives/list/python-dev@python.org/thread/OSKS23WDHU67NQCANPNSAPHATZ2J3W7T/
https://mail.python.org/archives/list/python-dev@python.org/thread/UJI3KJQVOY7VELTDJOBTBWM3TYDHEYGD/

The problems with the current behavior:

  1. It looks confusing that some of *Set* functions do removing, while other raise SystemError or crash. Ideally, all such functions should either support deletion by specifying NULL, or raise SystemError. A crash is acceptable too, for performance.
  2. It is a bug magnet. It is easy to get the NULL pointer instead of a pointer to object (just forgot to check for error) and delete instead of set.

Changing the current behavior is a long process. M.-A. Lemburg specified the required procedure. It is interesting, that if we started it in 3.6, it would already be finished.

vstinner commented Jun 9, 2023

Copy link
Copy Markdown
Member Author

I reported this error prone API as capi-workgroup/problems#47

vstinner commented Jun 9, 2023

Copy link
Copy Markdown
Member Author

It is a bug magnet. It is easy to get the NULL pointer instead of a pointer to object (just forgot to check for error) and delete instead of set.

Oh right. I would prefer to emit a DeprecationWarning in this case and prepare a migration plan towards "Del" functions instead. I don't think that a sentence in the documentation is enough to help users to avoid mistakes.

Copy link
Copy Markdown
Member Author

CAM-Gerlach changed the title gh-105373: PyObject_SetAttr(NULL) is no longer deprecated gh-105373: Remove the PyObject_SetAttr(NULL) deprecation Jun 17, 2023

vstinner commented Jul 9, 2023

Copy link
Copy Markdown
Member Author

I close this PR in favor of issue #106572: Deprecate PyObject_SetAttr(obj, name, NULL): PyObject_DelAttr(obj, name) must be used instead.

vstinner closed this Jul 9, 2023
vstinner deleted the keep_obj_setattr_null branch July 9, 2023 16:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review docs Documentation in the Doc dir skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL