8000 BUG: ensure passing ``np.dtype`` to itself doesn't crash by ngoldbaum · Pull Request #25042 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG: ensure passing np.dtype to itself doesn't crash #25042

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
merged 1 commit into from
Nov 1, 2023
Merged

BUG: ensure passing np.dtype to itself doesn't crash #25042

merged 1 commit into from
Nov 1, 2023

Conversation

ngoldbaum
Copy link
Member
@ngoldbaum ngoldbaum commented Oct 31, 2023 •
edited
Loading

Happy to implement other approaches but adding a check just before the crash happens seemed simplest.

Fixes #25031.

@ngoldbaum ngoldbaum added the 09 - Backport-Candidate PRs tagged should be backported label Oct 31, 2023
@ngoldbaum ngoldbaum force-pushed the fix-dtype-autophagia-segfault branch 3 times, most recently from 78d0619 to ac8f1f1 Compare October 31, 2023 16:10
@seberg
Copy link
Member
seberg commented Oct 31, 2023

It's a bit funny, I wonder if just flagging it as parametric would actually work :). To some degree, it is well defined (as in, it is identical to the user passing no dtype).
OTOH, in practice it should typically be a user error here. I do wonder if there is an earlier place to reject this nicely...

But, overall, I am also happy with just doing this, I don't think we would have to fret over ever changing it in the future anyway.

@ngoldbaum ngoldbaum force-pushed the fix-dtype-autophagia-segfault branch from ac8f1f1 to 0cb5982 Compare October 31, 2023 17:25
@ngoldbaum
Copy link
Member Author

I wonder if just flagging it as parametric would actually work :)

It turns out this makes the test I added create a float64 array, which I guess makes sense, there must be a default cast to float64 somewhere.

I do wonder if there is an earlier place to reject this nicely...

I ended up moving the check into PyArray_DTypeOrDescrConverterRequired so the error happens a little earlier.

@seberg
Copy link
Member
seberg commented Oct 31, 2023

Yeah, I think returning float64 there is actually completely fine after all that is what np.dtype() gives. Although I am not confident we end up there the right path.

Making this an error for now here seems fine then. If anyone ever feals to relax it to return the float64, I would not be opposed at all, though!

@ngoldbaum ngoldbaum force-pushed the fix-dtype-autophagia-segfault branch from 0cb5982 to b046293 Compare October 31, 2023 17:38
@seberg
Copy link
Member
seberg commented Nov 1, 2023

Thanks Nathan, let's go with this and possibly relax it if we ever find a reason for it.

@seberg seberg merged commit 95ca101 into numpy:main Nov 1, 2023
@charris charris changed the title BUG: ensure passing np.dtype to itself doesn't crash BUG: ensure passing np.dtype to itself doesn't crash Nov 12, 2023
@charris charris mentioned this pull request Nov 12, 2023
Merged
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
00 - Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BUG: segfault when passing dtype module to np.array constructor
3 participants
@ngoldbaum @seberg @charris
0