GeezerVison

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

GeezerVison is showing github.com — gh-106521: Add PyObject_GetOptionalAttr() function by serhiy-storchaka · Pull Request #106522 · python/cpython

Conversation

serhiy-storchaka commented Jul 7, 2023 •

edited by github-actions Bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Member

It is a new name of former _PyObject_LookupAttr().

Issue: C API: Add PyObject_GetOptionalAttr() function #106521

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

Sorry, something went wrong.

Uh oh!

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

All reactions


          pythongh-106521: Add PyObject_GetOptionalAttr() function

cf4d3cd

It is a new name of former _PyObject_LookupAttr().

serhiy-storchaka requested review from 1st1, abalkin, asvetlov, berkerpeksag, brettcannon, ericsnowcurrently, erlend-aasland, gvanrossum, iritkatriel, isidentical, kumaraditya303, markshannon, methane, ncoghlan, pganssle, rhettinger, warsaw and willingc as code owners

July 7, 2023 16:15

bedevere-bot added the awaiting core review label

Jul 7, 2023

bedevere-bot mentioned this pull request

Jul 7, 2023

C API: Add PyObject_GetOptionalAttr() function #106521

Closed


          Update Misc/stable_abi.toml.

3fb9775

serhiy-storchaka requested review from a team and encukou as code owners

July 7, 2023 18:33

carljm reviewed

Jul 7, 2023

View reviewed changes

Comment thread

Doc/c-api/object.rst Resolved

Uh oh!

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

Comment thread

Doc/c-api/object.rst Resolved

Uh oh!

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

Comment thread

Doc/whatsnew/3.13.rst Resolved

Uh oh!

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

Comment thread

Include/abstract.h Resolved

Uh oh!

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

brettcannon removed request for a team, brettcannon, erlend-aasland, isidentical and kumaraditya303

July 7, 2023 22:01

serhiy-storchaka requested review from vstinner and removed request for 1st1, abalkin, asvetlov, berkerpeksag, encukou, ericsnowcurrently, gvanrossum, iritkatriel, markshannon, methane, ncoghlan, pganssle, warsaw and willingc

July 10, 2023 12:59

vstinner reviewed

Jul 10, 2023

View reviewed changes

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.

To ease the review, you might keep the old name with an alias #define _PyObject_LookupAttr PyObject_GetOptionalAttr, and then write a second PR to update usage of the old name. (Well, just split the PR in two parts.)

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Doc/c-api/object.rst



		.. c:function:: int PyObject_GetOptionalAttrString(PyObject obj, const char attr_name, PyObject **result);

vstinner Jul 10, 2023

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.

You may just say:

Similar to :c:func:`PyObject_GetOptionalAttr`, but *name* is a UTF-8 encoded string.

To not copy/paste the documentation.

Sorry, something went wrong.

Uh oh!

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

All reactions

serhiy-storchaka Jul 11, 2023

Copy link

Copy Markdown

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

In all other functions with String suffix the documentation is repeated. If we are going to change this, it should be changed for all function at once.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Doc/c-api/object.rst Resolved

Uh oh!

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

Comment thread

Doc/c-api/object.rst Resolved

Uh oh!

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

Comment thread

Doc/whatsnew/3.13.rst Resolved

Uh oh!

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

serhiy-storchaka and others added 3 commits

July 11, 2023 10:34


          Apply suggestions from code review

be344ce

Co-authored-by: Victor Stinner <[email protected]>


          Update other documentation according to review.

0f286c0


          Minimize changes.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request

Jul 11, 2023


          Rewrite documentation according to review in python#106522.

d6a9204

vstinner approved these changes

Jul 11, 2023

View reviewed changes

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.

LGTM. Thanks for the update.

Sorry, something went wrong.

Uh oh!

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

All reactions

bedevere-bot added awaiting merge and removed awaiting core review labels

Jul 11, 2023

serhiy-storchaka commented Jul 11, 2023

Copy link

Copy Markdown

Member Author

Thank you for review @carljm and @vstinner, and especially for your documentation suggestions.

carljm and erlend-aasland reacted with hooray emoji

All reactions

🎉 2 reactions

Sorry, something went wrong.

Uh oh!

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


          Merge branch 'main' into PyObject_GetOptionalAttr

e9ffff1

serhiy-storchaka merged commit 579aa89 into python:main

Jul 11, 2023

bedevere-bot removed the awaiting merge label

Jul 11, 2023

serhiy-storchaka deleted the PyObject_GetOptionalAttr branch

July 11, 2023 19:13

cdce8p mentioned this pull request

Jul 7, 2024

Fix _PyObject_LookupAttrId for Python 3.13 python/mypy#17505

Merged

hauntsaninja pushed a commit to python/mypy that referenced this pull request

Jul 7, 2024


          Fix _PyObject_LookupAttrId for Python 3.13 (#17505)

7f67090

`_PyObject_LookupAttrId` was removed / replaced with
`PyObject_GetOptionalAttrString` in
python/cpython#106522.

https://docs.python.org/dev/c-api/object.html#c.PyObject_GetOptionalAttrString

Fixes
```cpp
  /home/runner/work/mypy/mypy/mypyc/lib-rt/pythonsupport.h: In function ‘update_bases’: (diff)
  /home/runner/work/mypy/mypy/mypyc/lib-rt/pythonsupport.h:51:13: error: implicit declaration of function ‘_PyObject_LookupAttrId’; did you mean ‘_PyObject_GetAttrId’? [-Werror=implicit-function-declaration] (diff)
     51 |         if (_PyObject_LookupAttrId(base, &PyId___mro_entries__, &meth) < 0) { (diff)
        |             ^~~~~~~~~~~~~~~~~~~~~~ (diff)
        |             _PyObject_GetAttrId (diff)
```

Reviewers

carljm

carljm left review comments

vstinner

vstinner approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

4 participants

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.

GeezerVison

Conversation

serhiy-storchaka commented Jul 7, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner Jul 10, 2023

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Jul 11, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Jul 11, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

serhiy-storchaka commented Jul 7, 2023 •

edited by github-actions Bot

Loading

Uh oh!

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