Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #617 +/- ##
==========================================
- Coverage 96.29% 94.11% -2.19%
==========================================
Files 57 46 -11
Lines 2540 2414 -126
Branches 127 121 -6
==========================================
- Hits 2446 2272 -174
- Misses 71 121 +50
+ Partials 23 21 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a typing stub for unxt.quantity to help mypy accept Plum-parametric Quantity[...] usage by declaring AbstractParametricQuantity/Quantity as generics.
Changes:
- Introduces
src/unxt/quantity.pyidefiningAbstractQuantity,AbstractParametricQuantity, andQuantityas typed/generic stubs. - Adds operator method signatures intended to mirror
quax_blocksmixins for basic arithmetic typing.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <[email protected]> Signed-off-by: Adrian Price-Whelan <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #617 +/- ##
==========================================
- Coverage 96.29% 96.18% -0.12%
==========================================
Files 57 46 -11
Lines 2540 2414 -126
Branches 127 121 -6
==========================================
- Hits 2446 2322 -124
+ Misses 71 70 -1
+ Partials 23 22 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nstarman
left a comment
There was a problem hiding this comment.
If this fixes mypy errors then that's good. But does this limit ty's inference?
nstarman
left a comment
There was a problem hiding this comment.
I think this is correct. It annotates the set types while keeping the init general
ty is borked for me on any parametric Quantity, but I don't understand why. Potentially related to: astral-sh/ty#2294 |
Co-authored-by: Nathaniel Starkman <[email protected]> Signed-off-by: Adrian Price-Whelan <[email protected]>
Co-authored-by: Nathaniel Starkman <[email protected]> Signed-off-by: Adrian Price-Whelan <[email protected]>
nstarman
left a comment
There was a problem hiding this comment.
We should add a ty job to CI.
When I use parametric Quantity's for typing, I get loads of mypy errors like "type[Quantity] is not generic and not indexable". I think this is because mypy only recognizes subscriptability for classes that inherit from
typing.Generic, so it doesn't understandplum.parametric's__class_getitem__at runtime.This adds a minimal .pyi stub declaring
QuantityandAbstractParametricQuantityasGeneric[_T](_T: LiteralString), plus most of the relevant operators inherited from thequax_blocksmixins. I think with this, mypy should acceptQuantity["time"]and treatQuantity["time"]andQuantity["length"]as distinct types.This is minimal so far and doesn't replicate the full mixin hierarchy, but I think that's ok for type checking purposes?
(btw: ty handles
plum.parametriccorrectly without the stub, so I guess this is specific to mypy?)