Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jun 6, 2023

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
Copy link
Member Author

vstinner commented Jun 6, 2023

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.

@serhiy-storchaka
Copy link
Member

What is changed since it was discussed 8 years ago?

@vstinner
Copy link
Member Author

vstinner commented Jun 6, 2023

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

@serhiy-storchaka
Copy link
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
Copy link
Member Author

vstinner commented Jun 9, 2023

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

@vstinner
Copy link
Member Author

vstinner commented Jun 9, 2023

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.

@vstinner
Copy link
Member Author

@CAM-Gerlach 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
Copy link
Member Author

vstinner commented Jul 9, 2023

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

@vstinner vstinner closed this Jul 9, 2023
@vstinner vstinner deleted the keep_obj_setattr_null branch July 9, 2023 16:09
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