Repository navigation
Conversation
Member
|
Hmmm thanks. My quick thought is that I am a bit unsure. I omitted it initially intentionally, because:
OTOH, yeah, unless we are unsure about |
Member
Author
|
I'm happy to leave the legacy stuff alone too. I just thought the version that net deletes code is better. I'll let you think about it, no rush. |
Member
|
Well, it net deletes code by adding API promises :). |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Generate missing self-casts for DTypes using
NPY_DT_legacy_descriptor_protofrom their legacy copyswap implementations. This fixes a segfault triggerable by comparing arational2instance with the byteswapped version, found in review of #33019. I ended up net negative on lines of code because it replacesrational2’s custom self-cast.I think that means eventually
ml_dtypesmight be able to use the automatically defined self cast (ping @YukaSadaoka). In principle I think we could also backport it with a shim innpy_2_compat.hin a followup.Also fixes legacy user dtypes incorrectly allowing byte-order changes with
casting="no", adds regression coverage for both rational dtypes, and clarifiesdtype(copy=True)behavior (as requested by @seberg in #33019).AI Disclosure
An AI model found the segfault and I iterated with it on the PR.