-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat!: harden dtype handling for scalars #418
feat!: harden dtype handling for scalars #418
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_object
function, 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 :)