Skip to content

fix: using numba.core.errors.Numba<Error> instead of Error in a Numba typing context.#2659

Merged
jpivarski merged 3 commits intomainfrom
jpivarski/adapt-to-numba-0.58rc1
Aug 21, 2023
Merged

fix: using numba.core.errors.Numba<Error> instead of Error in a Numba typing context.#2659
jpivarski merged 3 commits intomainfrom
jpivarski/adapt-to-numba-0.58rc1

Conversation

@jpivarski
Copy link
Copy Markdown
Member

This is the only update needed to pass tests with Numba 0.58.0rc1. However, that version of Numba still raises a warning if

export NUMBA_CAPTURED_ERRORS=new_style

is not explicitly set (should probably be changed in Numba itself). Most of the exceptions changed were TypeErrors, but a few were ValueErrors that really ought to have been type errors. (Not finding a record field by name is a type error in compiled code.)

Vector is already up-to-date with respect to TypeError versus numba.TypingError.

@jpivarski jpivarski requested a review from ianna August 18, 2023 18:32
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 18, 2023

Codecov Report

Merging #2659 (01e1dd9) into main (01cbec2) will increase coverage by 0.00%.
Report is 1 commits behind head on main.
The diff coverage is 32.00%.

Additional details and impacted files
Files Changed Coverage Δ
src/awkward/_connect/numba/builder.py 84.95% <15.00%> (+0.03%) ⬆️
src/awkward/_connect/numba/layout.py 83.68% <16.66%> (+0.02%) ⬆️
src/awkward/index.py 89.94% <42.85%> (-2.09%) ⬇️
src/awkward/_connect/numba/arrayview_cuda.py 35.29% <50.00%> (+4.04%) ⬆️
src/awkward/_connect/numba/layoutbuilder.py 87.27% <50.00%> (+0.01%) ⬆️
src/awkward/_connect/numba/arrayview.py 93.37% <100.00%> (+0.01%) ⬆️
src/awkward/operations/ak_from_dlpack.py 62.06% <100.00%> (+6.89%) ⬆️

@jpivarski jpivarski temporarily deployed to docs-preview August 18, 2023 18:42 — with GitHub Actions Inactive
@agoose77
Copy link
Copy Markdown
Collaborator

agoose77 commented Aug 21, 2023

From the deprecation notice,, it seems to me that we should be using the Numba-aliased errors. I replaced our changes to NumbaTypingError with the appropriate specific error class. Hopefully this works ...

I also therefore am mixing import styles, but I think in time we are moving to from XXX import YYY.

@agoose77 agoose77 changed the title fix: using numba.TypingError for normal/expected errors in a Numba typing context. fix: using numba.core.errors.Numba<Error> instead of Error in a Numba typing context. Aug 21, 2023
@agoose77 agoose77 temporarily deployed to docs-preview August 21, 2023 12:47 — with GitHub Actions Inactive
@jpivarski
Copy link
Copy Markdown
Member Author

I'm not really sure what the intended difference is between nb.TypingError and nb.NumbaTypeError. They both participate in the new-style of NUMBA_CAPTURED_ERRORS:

>>> issubclass(numba.TypingError, numba.NumbaError)
True
>>> issubclass(numba.NumbaTypeError, numba.NumbaError)
True

Oh, it turns out that nb.NumbaTypeError is just a subclass of nb.TypingError.

>>> numba.TypingError.mro()
[<class 'numba.core.errors.TypingError'>, <class 'numba.core.errors.NumbaError'>, <class 'Exception'>, <class 'BaseException'>, <class 'object'>]
>>> numba.NumbaTypeError.mro()
[<class 'numba.core.errors.NumbaTypeError'>, <class 'numba.core.errors.TypingError'>, <class 'numba.core.errors.NumbaError'>, <class 'Exception'>, <class 'BaseException'>, <class 'object'>]

I'm on board with from XXX import YYY, and in that style, NumbaTypeError is less ambiguous than TypingError would be. So I think these are fine counter-edits.

@ianna, what do you think?

Copy link
Copy Markdown
Member

@ianna ianna left a comment

Choose a reason for hiding this comment

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

I like it, it is more expressive. Thanks!

@jpivarski
Copy link
Copy Markdown
Member Author

Okay, then I'll merge!

@jpivarski jpivarski enabled auto-merge (squash) August 21, 2023 16:14
@jpivarski jpivarski temporarily deployed to docs-preview August 21, 2023 16:24 — with GitHub Actions Inactive
@jpivarski jpivarski merged commit a42e44c into main Aug 21, 2023
@jpivarski jpivarski deleted the jpivarski/adapt-to-numba-0.58rc1 branch August 21, 2023 16:27
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.

3 participants