feat: update to latest awkward==2.5.0
#407
Conversation
agoose77
force-pushed
the
agoose77/feat-add-attrs
branch
from
9ebaa37
to
4c7265d
Compare
| lists, | ||
| meta=typetracer_array(ak.Array(lists[0])), | ||
| meta=typetracer_array(ak.Array(lists[0], attrs=attrs, behavior=behavior)), |
There was a problem hiding this comment.
@douglasdavis hmm we might need to add a mechanism for passing a type object and/or enforcing this meta - lists could have any type in lists[1], etc.
There was a problem hiding this comment.
Yea I see what you're getting at here. from_lists is meant to be a pretty simple entry point to instantiating a dask-awkward array from non-disk data. I originally wrote it as a simple testing ground when adding the from_map interface and I felt like it wouldn't be a bad addition to the IO API. My thoughts here are to keep it simple and perhaps add a note to the docstring describing the function as expecting the same type for each list.
There was a problem hiding this comment.
Perhaps we can make it a runtime error if the subsequent partitions don't have the anticipated meta?
There was a problem hiding this comment.
That seems reasonable to me
|
There's a change in Awkward 2.4.9 that strictly requires |
agoose77
changed the title
attrsawkward==2.5.0
|
The conda test will fail until conda-forge's bot grabs the latest awkward so we can ignore that workflow |
| @@ -47,6 +47,7 @@ def test_list_with_ints_raise(daa: dak.Array) -> None: | |||
|
|
|||
|
|
|||
| def test_single_int(daa: dak.Array, caa: ak.Array) -> None: | |||
| daa.eager_compute_divisions() | |||
There was a problem hiding this comment.
@douglasdavis can you confirm this is a reasonable fix for the changes to implicit len computation?
There was a problem hiding this comment.
We probably want to avoid eager_compute_divisions on the fixture since it's long-lived, perhaps add a
daa = copy.copy(daa)above the divisions compute
There was a problem hiding this comment.
Ooh, lovely idea.
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #407 +/- ##
==========================================
+ Coverage 93.85% 93.91% +0.05%
==========================================
Files 23 23
Lines 3125 3153 +28
==========================================
+ Hits 2933 2961 +28
Misses 192 192 ☔ View full report in Codecov by Sentry. |
|
@douglasdavis I think this is ready to go. I haven't done a pass of adding new |
|
We need to add tests, but that's a bigger task. I am thinking towards us merging this to fix things, and do a patch release if we spot anything in our later tests. |
|
Just doing some last checks actually. |
|
OK, let's hold this, found some strange behavior. |
|
Thanks @douglasdavis. I found a bug in Awkward, so we can't fix that here. Yet, it's not a show-stopper. I've bumped our minimum version of awkward to 2.5.0. |
This PR adds support for
awkward==2.5.0's latest features, includingattrsand a public typetracer API.