Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
bpo-32571: Avoid raising unneeded AttributeError and silencing it in C code (alt).#5222
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
bpo-32571: Avoid raising unneeded AttributeError and silencing it in C code (alt).#5222
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…ode.Added _PyObject_GetAttrIdWithoutError() and used in appropriate cases.
…bject_LookupAttrId().
| Return -1 and set *result == NULL if an error other than AttributeError | ||
| is raised. | ||
| */ | ||
| PyAPI_FUNC(int) _PyObject_LookupAttr(PyObject *, PyObject *, PyObject **); |
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.
I would rename the method toPyObject_LookupAttr (no leading underscore). I'd use it in my libraries and the leading underscore would make me think that I shouldn't.
I also like Inada-san's suggestion to simplify the return value even further:
-1, if anunrelated error occurred during attribute lookup0if no errors have occurred;*resultwill be set toNULLif the attribute was not found, and to a non-NULLvalue if it was found.
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.
I think this API is experimental and should be kept private for some time. We are still discussing it, and after making some design decision we may change it in future.
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.
I agree with@serhiy-storchaka.
While I feel this API is nice, I'm not sure we should set*result=NULL when returning -1 or not.
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.
Don't we care C API methods with a leading underscore the same as without? I've never seen us breaking the_ prefixed API to be honest. So leading underscore or not it's set in stone.
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.
@1st1 Actually speaking, I broke_PyObject_GenericGetAttrWithDict(). I added one argument to suppress AttributeError.
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.
Fair enough. The PR is LGTM.
@serhiy-storchaka Would you merge this? |
I guess we can merge this ourselves if Serhiy isn't around. I'd like us to have this in 3.7. |
Uh oh!
There was an error while loading.Please reload this page.
Add
_PyObject_LookupAttr()and_PyObject_LookupAttrId()and use them in appropriate cases instead ofPyObject_GetAttr(),_PyObject_GetAttrId()and_PyObject_GetAttrIdWithoutError().https://bugs.python.org/issue32571