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 dak.where #133

Merged
merged 5 commits into from
Jan 2, 2023
Merged

Add dak.where #133

merged 5 commits into from
Jan 2, 2023

Conversation

lgray
Copy link
Collaborator

@lgray lgray commented Dec 16, 2022

Fixes #127

@codecov-commenter
Copy link

codecov-commenter commented Dec 17, 2022

Codecov Report

Merging #133 (3365e72) into main (78e5712) will decrease coverage by 0.03%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##             main     #133      +/-   ##
==========================================
- Coverage   95.72%   95.69%   -0.04%     
==========================================
  Files          18       18              
  Lines        1754     1765      +11     
==========================================
+ Hits         1679     1689      +10     
- Misses         75       76       +1     
Impacted Files Coverage Δ
src/dask_awkward/lib/structure.py 96.22% <91.66%> (-0.30%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lgray
Copy link
Collaborator Author

lgray commented Dec 18, 2022

It occurs to me that we could make it so the meta is known ahead of time because ak.where most often implies a return value of ?? * outer * structure * union[the,input,types], where the union type can be simplified. However, ak.where does not function on typetracers. Presently returns an error like this:

>>> ak.where((dx > 2)._meta, dx._meta, dx._meta)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/lgray/miniforge3/lib/python3.9/site-packages/awkward/operations/ak_where.py", line 61, in where
    return _impl3(condition, x, y, mergebool, highlevel, behavior)
  File "/Users/lgray/miniforge3/lib/python3.9/site-packages/awkward/operations/ak_where.py", line 132, in _impl3
    out = ak._broadcasting.broadcast_and_apply(
  File "/Users/lgray/miniforge3/lib/python3.9/site-packages/awkward/_broadcasting.py", line 1022, in broadcast_and_apply
    out = apply_step(
  File "/Users/lgray/miniforge3/lib/python3.9/site-packages/awkward/_broadcasting.py", line 1001, in apply_step
    return continuation()
  File "/Users/lgray/miniforge3/lib/python3.9/site-packages/awkward/_broadcasting.py", line 723, in continuation
    outcontent = apply_step(
  File "/Users/lgray/miniforge3/lib/python3.9/site-packages/awkward/_broadcasting.py", line 1001, in apply_step
    return continuation()
  File "/Users/lgray/miniforge3/lib/python3.9/site-packages/awkward/_broadcasting.py", line 766, in continuation
    outcontent = apply_step(
  File "/Users/lgray/miniforge3/lib/python3.9/site-packages/awkward/_broadcasting.py", line 987, in apply_step
    result = action(
  File "/Users/lgray/miniforge3/lib/python3.9/site-packages/awkward/operations/ak_where.py", line 111, in action
    tags = ak.index.Index8((npcondition == 0).view(np.int8))
AttributeError: 'bool' object has no attribute 'view'

where dx is a dask-awkward array. @jpivarski do you think it would be possible to get ak.where to predict the return type based on a typetracer?

@jpivarski
Copy link
Collaborator

The above should now be fixed in Awkward main.

@lgray
Copy link
Collaborator Author

lgray commented Dec 20, 2022

@jpivarski I confirm in Awkward main that dak.where no longer requires running the first chunk to determine array metadata and functions correctly on typetracers.

@lgray
Copy link
Collaborator Author

lgray commented Dec 23, 2022

This should now pass CI if run, with the release of awkward 2.0.3!

@lgray
Copy link
Collaborator Author

lgray commented Jan 2, 2023

Could someone kick this PR for tests? Thanks!

@martindurant
Copy link
Collaborator

Rerunning all tests

@martindurant
Copy link
Collaborator

(I think pushing an empty commit is actually the easiest way to do that!)

@lgray
Copy link
Collaborator Author

lgray commented Jan 2, 2023

@martindurant the tests will not run automatically for me yet since I have not successfully completed a PR to this repository. Some branch/repository protection rule.

@martindurant martindurant merged commit 5622059 into dask-contrib:main Jan 2, 2023
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.

dak.where
5 participants