Skip to content

gh-117398: Add datetime C-API type check test for subinterpreters#119604

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
ericsnowcurrently merged 40 commits into from
Jun 13, 2024
Merged

gh-117398: Add datetime C-API type check test for subinterpreters #119604

ericsnowcurrently merged 40 commits into from
Jun 13, 2024

Conversation

@neonene
Copy link
Contributor

@neoneneneonene commented May 27, 2024•
edited
Loading

Check if the DateType C-API type matches the datetime.date type on main and shared/isolated subinterpreters.

@ericsnowcurrently Please review if this usage is not invalid.

@bedevere-appbedevere-appbot added tests Tests in the Lib/test dir awaiting review labels May 27, 2024
@bedevere-appbedevere-appbot mentioned this pull request May 27, 2024
Closed
ericsnowcurrently
ericsnowcurrently requested changes May 27, 2024
View reviewed changes
Copy link
Member

@ericsnowcurrentlyericsnowcurrently left a comment

Choose a reason for hiding this comment

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

The test makes sense. Thanks for adding it. I have one question.

@bedevere-app
Copy link

bedevere-appbot commented May 27, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@neonene
Copy link
ContributorAuthor

neonene commented May 28, 2024

I have made the requested changes; please review again.

@bedevere-app
Copy link

bedevere-appbot commented May 28, 2024

Thanks for making the requested changes!

@ericsnowcurrently: please review the changes made to this pull request.

@neoneneneonene changed the title gh-117398: Add test for datetime C API type checkgh-117398: Add C-API test for datetime static type checkMay 28, 2024
@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented May 28, 2024

It looks like part of your PR was removed by mistake. I thought the new test in test_capi.test_misc would still be there.

@neoneneneonene changed the title gh-117398: Add C-API test for datetime static type checkgh-117398: Add datetime C-API type check test for subinterpretersMay 29, 2024
ericsnowcurrently
ericsnowcurrently requested changes Jun 13, 2024
View reviewed changes
Copy link
Member

@ericsnowcurrentlyericsnowcurrently left a comment

Choose a reason for hiding this comment

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

Thanks for moving that test! There's just one thing to address and then this PR should be ready to merge. Thanks for your patience.

@bedevere-app
Copy link

bedevere-appbot commented Jun 13, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@neonene
Copy link
ContributorAuthor

neonene commented Jun 13, 2024

I have made the requested changes; please review again.

@bedevere-app
Copy link

bedevere-appbot commented Jun 13, 2024

Thanks for making the requested changes!

@ericsnowcurrently: please review the changes made to this pull request.

ericsnowcurrently
ericsnowcurrently reviewed Jun 13, 2024
View reviewed changes
ericsnowcurrently
ericsnowcurrently approved these changes Jun 13, 2024
View reviewed changes
Copy link
Member

@ericsnowcurrentlyericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

@ericsnowcurrentlyericsnowcurrently merged commit 50a3895 into python:mainJun 13, 2024
@miss-islington-app
Copy link

miss-islington-appbot commented Jun 13, 2024

Thanks @neonene for the PR, and @ericsnowcurrently for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 13, 2024
@neonene@miss-islington
pythongh-117398: Add datetime C-API type check test for subinterprete…
8fb3ea8
…rs (pythongh-119604) Check if the DateTime C-API type matches the datetime.date type on main and shared/isolated subinterpreters. (cherry picked from commit 50a3895) Co-authored-by: neonene <53406459+neonene@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-appbot commented Jun 13, 2024

GH-120463 is a backport of this pull request to the 3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13 bugs and security fixes label Jun 13, 2024
@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Jun 13, 2024

Thanks again for all your work on this, @neonene!

@neoneneneonene deleted the capi_type_check branch June 13, 2024 18:10
ericsnowcurrently pushed a commit that referenced this pull request Jun 13, 2024
@miss-islington@neonene
[3.13]gh-117398: Add datetime C-API type check test for subinterpret…
15c3d00
…ers (gh-120463) Check if the DateTime C-API type matches the datetime.date type on main and shared/isolated subinterpreters. (cherry picked from commit 50a3895, AKA gh-119604) Co-authored-by: neonene <53406459+neonene@users.noreply.github.com>
@freakboy3742freakboy3742 mentioned this pull request Jun 14, 2024
Merged
mrahtz pushed a commit to mrahtz/cpython that referenced this pull request Jun 30, 2024
@neonene@mrahtz
pythongh-117398: Add datetime C-API type check test for subinterprete…
2129fa4
…rs (pythongh-119604) Check if the DateTime C-API type matches the datetime.date type on main and shared/isolated subinterpreters.
@neoneneneonene mentioned this pull request Jul 10, 2024
Merged
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
@neonene@noahbkim
pythongh-117398: Add datetime C-API type check test for subinterprete…
78a5805
…rs (pythongh-119604) Check if the DateTime C-API type matches the datetime.date type on main and shared/isolated subinterpreters.
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@neonene@estyxx
pythongh-117398: Add datetime C-API type check test for subinterprete…
6285938
…rs (pythongh-119604) Check if the DateTime C-API type matches the datetime.date type on main and shared/isolated subinterpreters.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericsnowcurrentlyericsnowcurrentlyericsnowcurrently approved these changes

@markshannonmarkshannonAwaiting requested review from markshannon

@gvanrossumgvanrossumAwaiting requested review from gvanrossum

@pgansslepganssleAwaiting requested review from pgansslepganssle is a code owner

@abalkinabalkinAwaiting requested review from abalkinabalkin is a code owner

Assignees

No one assigned

Labels

skip newstestsTests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@neonene@ericsnowcurrently@encukou