fix: give a useful error when ProjectionClass*D is missing - #737
fix: give a useful error when ProjectionClass*D is missing#737FanxinSun wants to merge 3 commits into
Conversation
If you build a subclass from the VectorAwkward mixins and don't give it a ProjectionClass3D/4D (like coffea's VertexArray, which intentionally has no 4D meaning), any operation that needs to promote the result blows up with a confusing AttributeError from awkward's field lookup that says nothing about what's actually missing. Route the ProjectionClass lookups in the awkward backend through a small helper that says which class is missing which attribute and what to do about it. Fixes scikit-hep#488
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #737 +/- ##
=======================================
Coverage 87.72% 87.73%
=======================================
Files 96 96
Lines 11194 11201 +7
=======================================
+ Hits 9820 9827 +7
Misses 1374 1374 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Saransh-cpp
left a comment
Saransh-cpp
left a comment
There was a problem hiding this comment.
Thanks a lot for working on this, @FanxinSun! As with your other PR: Please see #729 (comment) for my take on AI generated comments / code. I'll be happy to review this PR further if there is minimal AI usage (given that it is a good first issue).
| cls: type[VectorProtocol], dimension: int | ||
| ) -> type[VectorProtocol]: | ||
| name = f"ProjectionClass{dimension}D" | ||
| projection: type[VectorProtocol] | None = getattr(cls, name, None) |
There was a problem hiding this comment.
Is the type explicitly required here? Does mypy complain?
There was a problem hiding this comment.
I checked with:
mypy: 2.1.0
Python: 3.14.4
it will not complain. And this type annotation is not required, but one caveat is about warn_unreachable. Currently it's set false, by flipping it the un-annotated function will raise
error: Statement is unreachable [unreachable]
because ProjectionClass4D is declared non-Optional.
But in general I'm not clear about the plan of that flag so keeping it or not is up to you!
2 participants
Closes #488.
If you build a vector subclass from the awkward mixins and don't give it a
ProjectionClass3D/4D(like coffea'sVertexArray, which intentionally has no 4D meaning), any operation that tries to promote the result currently dies with an unhelpfulAttributeErrorfrom awkward's field lookup, which says nothing about what's actually missing.This PR routes the
ProjectionClass*Dlookups in the awkward backend through a small helper that raises aTypeErrornaming the class, the missing attribute, and how to fix it:Also adds a regression test that builds a minimal mixin-only
Vertexbehavior the same way coffea does. Full awkward backend test file passes locally (51 passed).