Skip to content

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

Merged
merged 6 commits into from
Mar 17, 2023
Merged

Implement isdtype() #32

merged 6 commits into from
Mar 17, 2023

Conversation

asmeurer
Copy link
Member

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.

if kind == 'bool':
return dtype == torch.bool
elif kind == 'signed integer':
return dtype in _int_dtypes and dtype.is_signed
Copy link
Member Author

@asmeurer asmeurer Mar 14, 2023

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?

Copy link
Member

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).

Copy link
Member Author

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
Copy link
Member Author

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?

Copy link
Member

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.

@asmeurer asmeurer merged commit e6007fa into data-apis:main Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Data Type Function, isdtype, from the v2022 spec
2 participants