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

Add support for casting from int enums #7581

Merged
merged 4 commits into from Nov 29, 2021

Conversation

testhound
Copy link
Contributor

@testhound testhound commented Nov 18, 2021

This PR fixes a enum to int cast problem documented in this issue: #7573 and introduced here: f2311d2.

Fixes #7573

@testhound
Copy link
Contributor Author

gpuci run tests

@gmarkall
Copy link
Member

Can you add a test for the failing case please?

@stuartarchibald stuartarchibald added this to the Numba 0.55 RC milestone Nov 19, 2021
@stuartarchibald stuartarchibald added the Effort - short Short size effort needed label Nov 19, 2021
@stuartarchibald stuartarchibald added 4 - Waiting on author Waiting for author to respond to review and removed 2 - In Progress labels Nov 19, 2021
@gmarkall
Copy link
Member

For the location of the test, it could go in numba/tests/test_enums in the TestIntEnum class.

@stuartarchibald stuartarchibald added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Nov 22, 2021
Copy link
Member

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

Thanks for adding the tests. I suggested some changes because there's already a test testing implicit coercion, so the uses of the IntEnum in this new case might as well all be casts - I suggested adding different types to cast to as well to ensure that casting to different widths also works.

@@ -50,6 +50,13 @@ def int_coerce_usecase(x):
else:
return x + Shape.circle

def int_cast_usecase(x):
# Implicit coercion of intenums to ints
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Implicit coercion of intenums to ints
# Explicit casting of intenums to ints

if x > RequestError.internal_error:
return x - RequestError.not_found
else:
return x + int32 (Shape.circle)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return x + int32 (Shape.circle)
return x + int8(Shape.circle)

Comment on lines 55 to 56
if x > RequestError.internal_error:
return x - RequestError.not_found
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if x > RequestError.internal_error:
return x - RequestError.not_found
if x > int16(RequestError.internal_error):
return x - int32(RequestError.not_found)

@@ -5,7 +5,7 @@

import numpy as np
import unittest
from numba import jit, vectorize
from numba import jit, vectorize, int32
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from numba import jit, vectorize, int32
from numba import jit, vectorize, int8, int16, int32

@gmarkall gmarkall added 4 - Waiting on author Waiting for author to respond to review and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Nov 25, 2021
@testhound
Copy link
Contributor Author

gpuci run tests

Copy link
Member

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

Thanks for the update - looks good to me!

@gmarkall gmarkall added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on author Waiting for author to respond to review Effort - short Short size effort needed labels Nov 29, 2021
@stuartarchibald stuartarchibald added the Effort - short Short size effort needed label Nov 29, 2021
@sklam sklam merged commit 6e2168a into numba:master Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to merge Review and testing done, is ready to merge Effort - short Short size effort needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

numba 0.54.1 casting IntEnum regression
4 participants