Skip to content

WIP,BUG: ensure that casting to void is properly checked. #13646

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

Closed

Conversation

mhvk
Copy link
Contributor

@mhvk mhvk commented May 27, 2019

As noted in #11114, currently the following clearly wrong things happen:

np.can_cast(1., 'i2,i2', casting='safe')
# True
# while
np.can_cast(1., 'i2', casting='safe')
# False
np.array(1000.).astype('i1,i1', casting='safe')
# array((-24, -24), dtype=[('f0', 'i1'), ('f1', 'i1')])
# while
np.array(1000.).astype('i1', casting='safe')
# TypeError: Cannot cast array from dtype('float64') to dtype('int8') according to the rule 'safe'

The present PR fixes it, and doesn't cause errors locally, but that is mostly because no tests are doing with casting to/from void. I'd happily add those, but what should and should not work?

EDIT: Should add that I'm not at all sure this is the right solution. Perhaps more likely the check should be different whenever from or to has fields.

fixes #11114

if (i != NPY_DATETIME) {
/*
* Bool -> <Anything> except datetime (since
* it conceptually has no zero)
*/
_npy_can_cast_safely_table[NPY_BOOL][i] = 1;
}
/* <Anything> -> Object */
/* <Anything> -> Object is possible */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Out of scope, but is this true for object->object?

}

_npy_can_cast_safely_table[NPY_STRING][NPY_UNICODE] = 1;
_npy_can_cast_safely_table[NPY_BOOL][NPY_TIMEDELTA] = 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is already included in the loop above.

Copy link
Member

Choose a reason for hiding this comment

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

Do you want to make a follow-up patch that removes this then, since you closed this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I felt it was too small, but you're right, might as well: #13652

@mhvk
Copy link
Contributor Author

mhvk commented May 28, 2019

Closing in favour of #13648, as I've convinced myself this was going in the wrong direction, changing things for the general void type rather than for structured arrays, which are one use of a void type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG? anything is allowed to cast to structured dtypes?
2 participants