fix: touch up serialization, add TypedDict#151
Conversation
Signed-off-by: Henry Schreiner <[email protected]>
There was a problem hiding this comment.
Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.
Files not reviewed (2)
- src/uhi/resources/histogram.schema.json: Language not supported
- tests/resources/reg.json: Language not supported
Comments suppressed due to low confidence (1)
src/uhi/typing/serialization.py:92
- [nitpick] The class name 'WeighedData' may be better spelled as 'WeightedData' to maintain consistency with similar names like 'WeightedMeanData'.
class WeighedData(TypedDict):
Signed-off-by: Henry Schreiner <[email protected]>
Thanks, Copilot! |
|
@jpivarski, @HDembinski, since this is a minor change to the schema (circular now a required property), can I get a signoff from you? |
|
Sounds good to me. I wondered for a moment if this might be an issue in Uproot because it doesn't mention the |
|
Also looks good to me. |
|
Yes, no one uses it yet, so I think it's safe to change a tiny bit as we implement it (plus the description warns it could change while being implemented). I'm working on getting it implemented in boost-histogram, based on @aryamanjeendgar's excellent start, and the ROOT team might be looking at it soon, after implementing UHI. Thanks! |
3 participants
One change:
circularis now a required property. Simplified implementation; I don't think there's a need to make it optional for a new standard. Metadata needs to be optional, as a way to indicate no metadata present, but circular is just like all the other non-optional fields.One fix:
And a feature:
total=Falsemechanism. It could be updated toRequired/NonRequiredin the future.