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

fix: Record is not in contents, but in record #1739

Merged
merged 2 commits into from
Sep 27, 2022
Merged

Conversation

ianna
Copy link
Collaborator

@ianna ianna commented Sep 27, 2022

This is popped up during CUDA testing: ak.contents.Record is in ak.record.Record

@codecov
Copy link

codecov bot commented Sep 27, 2022

Codecov Report

Merging #1739 (b34a445) into main (e692946) will increase coverage by 1.01%.
The diff coverage is n/a.

Additional details and impacted files
Impacted Files Coverage Δ
src/awkward/__init__.py 97.05% <ø> (ø)
src/awkward/_broadcasting.py 93.41% <ø> (ø)
src/awkward/_connect/avro.py 87.17% <ø> (ø)
src/awkward/_connect/cling.py 24.90% <ø> (ø)
src/awkward/_connect/cuda/__init__.py 0.00% <ø> (ø)
src/awkward/_connect/jax/__init__.py 90.47% <ø> (ø)
src/awkward/_connect/jax/_reducers.py 76.92% <ø> (ø)
src/awkward/_connect/numba/arrayview.py 97.77% <ø> (ø)
src/awkward/_connect/numba/builder.py 81.60% <ø> (ø)
src/awkward/_connect/numba/layout.py 84.87% <ø> (ø)
... and 311 more

@agoose77
Copy link
Collaborator

Huh I remember grepping for these, that's odd. Nice catch!

@agoose77
Copy link
Collaborator

agoose77 commented Sep 27, 2022

It seems like only the nplike module had actual code that was trying to resolve ak.contents.Record. @ianna did you hit this at runtime today? If so, could you add a test that reproduces it? Clearly we're not covering these nplike codepaths at the moment.

@ianna
Copy link
Collaborator Author

ianna commented Sep 27, 2022

It seems like only the nplike module had actual code that was trying to resolve ak.contents.Record. @ianna did you hit this at runtime today? If so, could you add a test that reproduces it? Clearly we're not covering these nplike codepaths at the moment.

Yes, we already have them: in tests-cuda

@agoose77 agoose77 merged commit 27e7dc1 into main Sep 27, 2022
@agoose77 agoose77 deleted the ianna/fix-cuda-tests branch September 27, 2022 18:12
@agoose77
Copy link
Collaborator

This just means that my CUDA tests weren't running locally, which is ... even worse than if we'd just missed a test case! 😢

Thanks!

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.

2 participants