Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-76595: PyCapsule_Import() now imports submodules if needed.#6898
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
base:main
Are you sure you want to change the base?
gh-76595: PyCapsule_Import() now imports submodules if needed.#6898
Conversation
Do you plan to add tests, or are you OK leaving that to someone else? |
Currently there are no any tests for PyCapsule. I'm going to write them if possible, but this will take a time. |
I'm asking because writing tests would make a nice task for mentoring someone – but it would take even more time that way. |
Objects/capsule.c Outdated
| } | ||
| } | ||
| } else { | ||
| if (object) { | ||
| PyObject *object2 = PyObject_GetAttrString(object, trace); | ||
| Py_DECREF(object); | ||
| object = object2; |
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.
Py_SETREF?
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.
Yes, it can be used here.
Objects/capsule.c Outdated
| if (no_block) { | ||
| object = PyImport_ImportModuleNoBlock(name_dup); | ||
| } else { | ||
| object = PyImport_ImportModule(name_dup); |
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.
aren't PyImport_ImportModuleNoBlock and PyImport_ImportModule the same thing now?
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.
Good point!
| Py_DECREF(object); | ||
| object = object2; | ||
| if (object) { | ||
| Py_SETREF(object, PyObject_GetAttrString(object, trace)); |
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 expect error handling here, if PyObject_GetAttrString() fails for whatever reason. It seems like PyImport_ImportModule() is tried on the substring on error.
bedevere-bot commentedOct 19, 2018
When you're done making the requested changes, leave the comment: |
@serhiy-storchaka, it looks like this one was close to being merged, so just putting it back on the radar. |
This PR is stale because it has been open for 30 days with no activity. |
I have a use case for this and would be happy to rebase / take over from@serhiy-storchaka if you don't have the time for it |
This PR is stale because it has been open for 30 days with no activity. |
This PR is stale because it has been open for 30 days with no activity. |
There's a PR (pythonGH-6898) which changes this behavior (to be lesssurprising, I believe), but this seems to have stalled.See:python#76595See:python#6898
Uh oh!
There was an error while loading.Please reload this page.
https://bugs.python.org/issue32414