Skip to content

Fix get_datetimestruct_days overflow #56001

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

Merged
merged 12 commits into from
Aug 19, 2024
Merged

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Nov 16, 2023

More pre-cursor for UBSAN/ASAN


if (did_overflow) {
Copy link
Member Author

Choose a reason for hiding this comment

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

We could also just PD_CHECK_OVERFLOW every expression but that is potentially a lot of branching. This allows more calls to build up in a pipeline and just raises at the very end if needed

@mroeschke mroeschke added the Internals Related to non-user accessible pandas implementation label Nov 16, 2023
if (did_error) {
return -1;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

this is acquiring the gil, checking if an overflow happened, then releasing it? is there a nogil alternative?

id be more comfortable with this if we had a test where it made a difference

Copy link
Member Author

Choose a reason for hiding this comment

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

The GIL is only acquired when days == -1 because you need to check if that is a legitimate value or if a Python exception was thrown. This is essentially what except? -1 does in Cython

Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Jan 11, 2024
@mroeschke mroeschke removed the Stale label Aug 19, 2024
@mroeschke mroeschke added this to the 3.0 milestone Aug 19, 2024
@mroeschke mroeschke merged commit eac20cc into pandas-dev:main Aug 19, 2024
39 of 47 checks passed
@mroeschke
Copy link
Member

Thanks @WillAyd

@WillAyd WillAyd deleted the fix-dt-overflow branch August 27, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internals Related to non-user accessible pandas implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants