Skip to content
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

Closes #2505: Update resolve_scalar_dtype to handle uint and bigint #2532

Conversation

stress-tess
Copy link
Member

This PR (closes #2505) updates resolve_scalar_dtype to correctly handle bigint and uint scalars. It also updates places where this is handled differently to just call that function.

…nd bigint

This PR (closes Bears-R-Us#2505) updates resolve scalar dtype to correctly handle bigint and uint scalars. It also updates places where this is handled differently to just call that function.
Copy link
Contributor

@Ethan-DeBandi99 Ethan-DeBandi99 left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Contributor

@jaketrookman jaketrookman left a comment

Choose a reason for hiding this comment

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

Everything looks good to me. Could you just explain why we need to switch from int to int64 here.

@stress-tess
Copy link
Member Author

Everything looks good to me. Could you just explain why we need to switch from int to int64 here.

@jaketrookman yeah! so previously our message.py was using type(val).__name__ to determine the dtype of scalars and the dtype of elements within a list. Now that we've updated it to use resolve_scalar_dtype, it returns int64 instead of int this is nice because it matches the pdarray dtypes, but also it allows for uint64 and bigint.

So previously if you looked at the logs 2**63 - 1, 2**63 + 1, and 2**65 + 1 would all have dtype: int. But now they'll have int64, uint64, and bigint respectively

@Ethan-DeBandi99 Ethan-DeBandi99 added this pull request to the merge queue Jun 29, 2023
Merged via the queue into Bears-R-Us:master with commit 7282f0b Jun 29, 2023
9 checks passed
@stress-tess stress-tess deleted the 2505_uint/bigint_resolve_scalar_dtype branch June 29, 2023 16:09
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.

weirdness with resolve_scalar_dtype
3 participants