Skip to content

bpo-1635741: Port itertools module to multiphase initialization.#19044

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.

Sign up for GitHub

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

Jump to bottom
Merged
vstinner merged 4 commits into from
Mar 17, 2020
Merged

bpo-1635741: Port itertools module to multiphase initialization. #19044

vstinner merged 4 commits into from
Mar 17, 2020

Conversation

@corona10
Copy link
Member

@corona10corona10 commented Mar 17, 2020•
edited by bedevere-bot
Loading

@corona10corona10force-pushed the bpo-1635741-itertools branch from 223b1c0 to ce22af3CompareMarch 17, 2020 15:28
@corona10corona10force-pushed the bpo-1635741-itertools branch from ce22af3 to 6980d78CompareMarch 17, 2020 15:32
@corona10corona10 requested a review from vstinnerMarch 17, 2020 16:11
@corona10
Copy link
MemberAuthor

corona10 commented Mar 17, 2020

@shihai1991 Can you please take a look?

vstinner
vstinner approved these changes Mar 17, 2020
View reviewed changes
Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. But I will wait for @shihai1991 review ;-)

vstinner
vstinner reviewed Mar 17, 2020
View reviewed changes
@vstinner
Copy link
Member

vstinner commented Mar 17, 2020

You forgot a typelist[i], I fixed it ;-)

shihai1991
shihai1991 approved these changes Mar 17, 2020
View reviewed changes
Copy link
Member

@shihai1991shihai1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ;)

@corona10
Copy link
MemberAuthor

corona10 commented Mar 17, 2020

You forgot a typelist[i], I fixed it ;-)

Thanks :)

@shihai1991
Copy link
Member

shihai1991 commented Mar 17, 2020

there is one thing to be confirmed: race condition of identifier in typelists can be avoid too? no potential risk?

@corona10
Copy link
MemberAuthor

corona10 commented Mar 17, 2020

there is one thing to be confirmed: race condition of identifier in typelists can be avoid too? no potential risk?

Can you explain more concrete situation?

@vstinner
Copy link
Member

vstinner commented Mar 17, 2020

there is one thing to be confirmed: race condition of identifier in typelists can be avoid too? no potential risk?

Currently, the GIL magically protects C extensions against race conditions. But the GIL can be released when executing arbitrary code. I don't think that it's the case here.

@vstinner
Copy link
Member

vstinner commented Mar 17, 2020•
edited
Loading

Static types initialized by PyType_Ready() are bad. Types allocated on the heap using PyType_FromSpec() are better:
https://pythoncapi.readthedocs.io/type_object.html

But this should be addressed in a separated change ;-)

@vstinnervstinner merged commit 514c469 into python:masterMar 17, 2020
@kylotankylotanmannequin mentioned this pull request Sep 19, 2022
Closed
@corona10corona10 deleted the bpo-1635741-itertools branch February 15, 2024 01:41
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinnervstinnervstinner approved these changes

@shihai1991shihai1991shihai1991 approved these changes

@rhettingerrhettingerAwaiting requested review from rhettinger

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@corona10@vstinner@shihai1991@the-knights-who-say-ni@bedevere-bot