-
Notifications
You must be signed in to change notification settings - Fork 35
Implement isdtype() #32
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
if kind == 'bool': | ||
return dtype == torch.bool | ||
elif kind == 'signed integer': | ||
return dtype in _int_dtypes and dtype.is_signed |
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.
Is there a better way to test if a torch dtype is integral?
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.
There isn't. Based on dtype properties it would be:
not dtype.is_floating_point and not dtype.is_complex and dtype.is_signed and dtype != dtype.bool
which looks worse and may also not be foolproof. The check you have will need explicit updating if for example uint16
is added to PyTorch, but that's okay (nothing like that is in the pipeline AFAIK).
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.
Hopefully this function itself would be added to pytorch by then.
elif kind == 'integral': | ||
return dtype in _int_dtypes | ||
elif kind == 'real floating': | ||
return dtype.is_floating_point |
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.
Is this the correct way to check for torch dtype categories?
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.
This should be good, yes.
Fixes #27
I've added some rudimentary testing here in lieu of test suite support, and also testing that the implementation works with some non-spec dtypes.