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.

Sign up for GitHub

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

Jump to bottom

allow using IntEnumMember as array index #9230

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

allow using IntEnumMember as array index #9230

wants to merge 6 commits into from

Conversation

dlee992
Copy link
Contributor

@dlee992 dlee992 commented Oct 9, 2023

related with #7662. but fix the similar issue for np.array.

@esc esc added skip_release_notes Skip towncrier requirement 3 - Ready for Review labels Oct 10, 2023
@esc esc closed this Oct 10, 2023
@esc esc reopened this Oct 10, 2023
@dlee992
Copy link
Contributor Author

dlee992 commented Oct 10, 2023

This test config numba.numba (Linux py310_np123) seems failed at installing python packages..

@esc esc requested a review from gmarkall October 10, 2023 14:57
@gmarkall gmarkall removed the skip_release_notes Skip towncrier requirement label Oct 11, 2023
gmarkall
gmarkall requested changes Oct 11, 2023
View reviewed changes
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.

Many thanks for the PR - could you also add a check with an IntEnum where negative values are used in the Enum?

Also, this should have a release note adding (the skip_release_notes tag was erroneously added before) - create a new file called docs/upcoming_chanages/9230.improvement.rst following the format suggested in https://github.com/numba/numba/blob/main/docs/upcoming_changes/README.rst (you can also look at some of the other files in https://github.com/numba/numba/tree/main/docs/upcoming_changes for an idea).

@gmarkall gmarkall added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels Oct 11, 2023
@gmarkall gmarkall added this to the 0.59.0-rc1 milestone Oct 11, 2023
@dlee992
Copy link
Contributor Author

dlee992 commented Nov 19, 2023

Adding more tests and a simple doc as you suggested right now. @gmarkall

@esc esc 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 20, 2023
@stuartarchibald
Copy link
Contributor

@dlee992 thanks for the update. It looks like 45056af contains a lot of automated formatting changes which are "hiding" the proposed change, please could these be removed so that the review can continue? Many thanks.

@stuartarchibald stuartarchibald 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 27, 2023
@dlee992
Copy link
Contributor Author

dlee992 commented Dec 5, 2023

Updated. (with force-push..)

gmarkall
gmarkall reviewed Dec 8, 2023
View reviewed changes
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.

I'm finding that I can't make the tests added here fail on main - @dlee992 is there a circumstance in which the tests fail on main for you? (I'm using Python 3.10)

docs/upcoming_changes/9230.improvement.rst Outdated Show resolved Hide resolved
@dlee992
Copy link
Contributor Author

dlee992 commented Dec 8, 2023

Em, I met this issue on my customised 0.57.1. Perhaps another PR has already fixed this somehow?

@gmarkall
Copy link
Member

gmarkall commented Dec 8, 2023

Em, I met this issue on my customised 0.57.1. Perhaps another PR has already fixed this somehow?

I just realised what the problem with the test is - see the comment on the diff.

@dlee992
Copy link
Contributor Author

dlee992 commented Dec 12, 2023

Em, the error is NameError: name 'compile_isolated' is not defined. But it's imported at line 8.. Don't know why..
https://dev.azure.com/numba/numba/_build/results?buildId=16490&view=logs&j=7e7ebb60-d573-5e5d-b793-95b37f06385f&t=5ac25388-b2c0-5549-4759-b1f5889bad88&l=486

@gmarkall
Copy link
Member

It's because this was merge, which removed compile_isolated: #9330

@dlee992
Copy link
Contributor Author

dlee992 commented Dec 23, 2023 •
edited
Loading

It's because this was merge, which removed compile_isolated: #9330

Got it. Work on this now.

@dlee992
Copy link
Contributor Author

dlee992 commented Feb 8, 2024

This can be in review status again. gentle cc @gmarkall

@stuartarchibald stuartarchibald modified the milestones: 0.59.0-rc1, 0.60.0-rc1 Feb 9, 2024
@esc esc 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 Feb 27, 2024
@esc
Copy link
Member

esc commented Feb 27, 2024

This can be in review status again. gentle cc @gmarkall

Thank you for the heads up, it's done.

@gmarkall gmarkall modified the milestones: 0.60.0-rc1, 0.61.0-rc1 Apr 9, 2024
@neich
Copy link

neich commented Jul 30, 2024

Any chance to accept this PR? This would be really useful

Thanks!

@dlee992
Copy link
Contributor Author

dlee992 commented Jul 30, 2024

@neich. Hi, Thanks for your notice.

I think this is added into milestone 0.61.0-rc1, it should be reviewed and merged when core devs begin to take actions for all issues and PRs in this release candidate.

Perhaps Oct or Nov this year, I guess. But you can use this patch locally as well, then don't have to wait for an official release.

@neich
Copy link

neich commented Jul 30, 2024

@dlee992 That's great! In the meantime I will use the numpy.intp explicit conversion and wait for the 0.61 release to get rid of it.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmarkall gmarkall Awaiting requested review from gmarkall

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
4 - Waiting on reviewer Waiting for reviewer to respond to author
Projects
None yet
Milestone
0.61.0-rc1
Development

Successfully merging this pull request may close these issues.
5 participants
@dlee992 @stuartarchibald @gmarkall @esc @neich