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.
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
(dev): 3.11 testing #3923
(dev): 3.11 testing #3923
Changes from 12 commits
f777165
056965c
e29f7a9
574bd95
60a2ab8
5045e2e
78ff640
0d7c4b7
9887cdc
1cd2513
dfa1a22
9b0dad5
e0e5673
04d60f1
6f43cc4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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'm really concerned about inlining the two complex functions here, especially because each version introduces multiple variables which could clash in non-obvious way for one Python version or another. We don't know how this code will need to be evolved in the future, accidents seem likely. I think it would be much better to keep the functions separate, even if we need
#ifdef
in both places, above for the function definitions, and here (I didn't look enough to know).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.
A comment here would be useful (similar to Henry's comment from a few days ago):
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.
What has to be duplicated?
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 had this comment in mind:
#3923 (comment)
@henryiii wrote 14 days ago:
When I saw the email that Henry had already merged this PR I stopped looking further. Doing that now:
This seems to be what @henryiii was referring to (and the "Link" I had in mind):
Comparing that with the new
#else
(Python 3.11) block here in embed.h, it looks like something completely different. Apparently I misinterpreted Henry's comment. Sorry.Looking more, it seems this is @vstinner's response to the @henryiii's comment above:
Which makes me think adding this link would be useful:
Or maybe alternatively the link to Victor's comment.
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.
PyConfig API initializes the default value of sys.path. But Py_RunMain() then inserts a path to sys.path. Python 3.11 safe_path=1 disables this behavior:
It might be interesting to move more code changing sys.path into the Python initialization (PyConfig), but it's complicated.