◐ Shell
reader mode source ↗
Skip to content

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

Merged
ericsnowcurrently merged 40 commits into
python:mainfrom
neonene:capi_type_check
Jun 13, 2024
Merged

gh-117398: Add datetime C-API type check test for subinterpreters#119604
ericsnowcurrently merged 40 commits into
python:mainfrom
neonene:capi_type_check

Conversation

@neonene

@neonene neonene commented May 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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-app bedevere-app Bot added tests Tests in the Lib/test dir awaiting review labels May 27, 2024
@bedevere-app bedevere-app Bot mentioned this pull request May 27, 2024
Closed
ericsnowcurrently
ericsnowcurrently requested changes May 27, 2024
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

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

@bedevere-app

bedevere-app Bot commented May 27, 2024

Copy link
Copy Markdown

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 neonene requested review from abalkin and pganssle as code owners May 28, 2024 03:20
@neonene

neonene commented May 28, 2024

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again.

@bedevere-app

bedevere-app Bot commented May 28, 2024

Copy link
Copy Markdown

Thanks for making the requested changes!

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

@bedevere-app bedevere-app Bot requested a review from ericsnowcurrently May 28, 2024 03:45
@neonene neonene changed the title gh-117398: Add test for datetime C API type check May 28, 2024
@ericsnowcurrently

ericsnowcurrently commented May 28, 2024

Copy link
Copy Markdown
Member

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.

@neonene neonene changed the title gh-117398: Add C-API test for datetime static type check May 29, 2024
53 hidden items Load more…
@bedevere-app bedevere-app Bot requested a review from ericsnowcurrently June 13, 2024 10:28
ericsnowcurrently
ericsnowcurrently requested changes Jun 13, 2024
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

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

bedevere-app Bot commented Jun 13, 2024

Copy link
Copy Markdown

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

neonene commented Jun 13, 2024

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again.

@bedevere-app

bedevere-app Bot commented Jun 13, 2024

Copy link
Copy Markdown

Thanks for making the requested changes!

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

@bedevere-app bedevere-app Bot requested a review from ericsnowcurrently June 13, 2024 18:00
ericsnowcurrently
ericsnowcurrently approved these changes Jun 13, 2024
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

LGTM

@ericsnowcurrently ericsnowcurrently merged commit 50a3895 into python:main Jun 13, 2024
@miss-islington-app

miss-islington-app Bot commented Jun 13, 2024

Copy link
Copy Markdown

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

bedevere-app Bot commented Jun 13, 2024

Copy link
Copy Markdown

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

@ericsnowcurrently

ericsnowcurrently commented Jun 13, 2024

Copy link
Copy Markdown
Member

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

@neonene neonene 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>
@freakboy3742 freakboy3742 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.
@neonene neonene 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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericsnowcurrently ericsnowcurrently ericsnowcurrently approved these changes

@markshannon markshannon Awaiting requested review from markshannon

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

@pganssle pganssle Awaiting requested review from pganssle pganssle is a code owner

@abalkin abalkin

Assignees

No one assigned

Labels

skip news tests Tests 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