-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
BUG: Change dtype comparison in _view_is_safe to be independent of name fields. #13261
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
Conversation
Haven't looked at this in detail, but I think it might relate to #8514 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately this approach does not work for structured dtype with padding in them
@lukasbrauncom do you wish to move forward with this? It needs tests that fail before this PR and now pass, and those tests should address the problem of padding. |
@eric-wieser Is this impacted by your recent fixups? |
return ndtype.type | ||
return newdtype | ||
|
||
if oldtype.names or oldtype.names: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo, one of these should say newtype.
Should also use .names is not None
here and above.
@eric-wieser Want to fix this up? |
We discussed this briefly, and the backport candidate label seemed unnecessary. I am going to close this due to the fact that it stalled for a longer time and would at least need new tests. I also expect that the machinery/fixes from gh-19346 could be interesting to generalize this more, although they are only available in C and not quite aligned with this, I think checking |
I have changed the _view_is_safe in _internal.py such that the comparison of dtypes is independent of the name field in structured arrays. This fixes issue #13213