feat!: harden dtype handling for scalars #418
Conversation
douglasdavis
force-pushed
the
scalar-dtype-and-meta-handling
branch
from
1315bca
to
423bc04
Compare
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #418 +/- ##
==========================================
- Coverage 93.75% 93.55% -0.20%
==========================================
Files 23 23
Lines 3139 3152 +13
==========================================
+ Hits 2943 2949 +6
- Misses 196 203 +7 ☔ View full report in Codecov by Sentry. |
src/dask_awkward/lib/core.py
Outdated
| self._meta: Any = self._check_meta(meta) | ||
| if meta is not None and dtype is None: | ||
| self._meta = self._check_meta(meta) | ||
| self._dtype = np.dtype(self._meta.type.content.primitive) |
There was a problem hiding this comment.
If the meta always has a NumpyType type, we could also just use self._meta.layout.dtype. I think that might be slightly better, because it uses Awkward's dtype→primitive conversion.
|
OK, looks good (though I made a change)! It seems then that scalars are now always implemented as an |
Co-authored-by: Angus Hollands <goosey15@gmail.com>
Yes that's what I'm going for here! |
This PR hardens treatment of meta/dtypes for Scalar collections. It changes the
Scalar.__init__andnew_scalar_objectfunction, which should really only find use inside of dask-awkward, but it's still possible this can break things.The PR makes sure that all Scalars have a meta and a dtype, stemming from conversation with @agoose77 on Slack, so I'd be happy to get your feedback here :)