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: apply subset of fixes for downstream dask usage #2283

Merged
merged 9 commits into from
Mar 6, 2023

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented Mar 6, 2023

TL;DR

  • Adds cumsum to TypeTracer
  • Removes to_rectilinear from nplikes, replaces with layout.to_backend_array

@agoose77 agoose77 force-pushed the agoose77/fix-typetracer-bugs-dask branch from d5b1755 to 6cde4b3 Compare March 6, 2023 17:53
@agoose77 agoose77 marked this pull request as ready for review March 6, 2023 17:54
@agoose77 agoose77 requested a review from jpivarski March 6, 2023 17:54
@agoose77 agoose77 temporarily deployed to docs-preview March 6, 2023 18:06 — with GitHub Actions Inactive
Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

This looks good, although there's at least one use of to_rectilinear in the tests, which is where this is failing right now.

assert nplike.to_rectilinear(array).tolist() == ak_array.to_list()

Comment on lines -243 to +245
len(nextinputs[0]) + 1, dtype=np.int64
nextinputs[0].length + 1, dtype=np.int64
Copy link
Member

Choose a reason for hiding this comment

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

Good catch!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This looks good, although there's at least one use of to_rectilinear in the tests, which is where this is failing right now.

assert nplike.to_rectilinear(array).tolist() == ak_array.to_list()

I could have sworn I ran the test suite locally. Oh, well, thank goodness for CI!

Copy link
Member

Choose a reason for hiding this comment

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

It came up in the Linux-ROOT test; it might be a test that requires ROOT in the specific environment to run. Although I see that another one failed, too, so maybe not.

@jpivarski
Copy link
Member

In c569a71, I simply removed the failing test. It was testing an aspect of to_rectilinear, but since the function no longer exists, I'd say that the property we intended to test is now obsolete.

@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Merging #2283 (8716fc8) into main (495f592) will decrease coverage by 0.02%.
The diff coverage is 63.63%.

Additional details and impacted files
Impacted Files Coverage Δ
src/awkward/_nplikes/cupy.py 40.84% <ø> (-0.26%) ⬇️
src/awkward/_nplikes/jax.py 83.78% <ø> (+1.73%) ⬆️
src/awkward/_nplikes/numpy.py 62.50% <ø> (-1.50%) ⬇️
src/awkward/_nplikes/numpylike.py 73.80% <ø> (+0.06%) ⬆️
src/awkward/_nplikes/typetracer.py 73.82% <27.27%> (-0.59%) ⬇️
src/awkward/_connect/numpy.py 92.99% <100.00%> (ø)
src/awkward/contents/unionarray.py 84.06% <100.00%> (+0.02%) ⬆️
src/awkward/operations/ak_concatenate.py 96.22% <100.00%> (ø)
src/awkward/operations/ak_unflatten.py 95.52% <100.00%> (ø)

@jpivarski jpivarski enabled auto-merge (squash) March 6, 2023 19:46
@jpivarski jpivarski temporarily deployed to docs-preview March 6, 2023 19:47 — with GitHub Actions Inactive
@jpivarski jpivarski merged commit c372281 into main Mar 6, 2023
@jpivarski jpivarski deleted the agoose77/fix-typetracer-bugs-dask branch March 6, 2023 19:55
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