chore: pass repo review #219
Conversation
Codecov Report
@@ Coverage Diff @@
## main #219 +/- ##
=======================================
Coverage 82.56% 82.56%
=======================================
Files 96 96
Lines 10475 10475
=======================================
Hits 8649 8649
Misses 1826 1826
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This PR grew bigger than I thought. Most of the changes are due to the updated pre-commit configuration, but there are a few changes which I think should be reviewed once. I have marked them down below. @henryiii @jpivarski could you please review them once? Thanks!
| if issubclass(cls, Vector2D): # type: ignore[unreachable] | ||
| return "Momentum2D" # type: ignore[unreachable] | ||
| elif issubclass(cls, Vector3D): # type: ignore[unreachable] | ||
| return "Momentum3D" # type: ignore[unreachable] | ||
| elif issubclass(cls, Vector4D): # type: ignore[unreachable] | ||
| return "Momentum4D" # type: ignore[unreachable] |
There was a problem hiding this comment.
type: ignore[unreachable]
There was a problem hiding this comment.
Guessing it is not understanding that Type[VectorProtocol] can (sometimes) pass a subclass of Momentum check? Might be worth updating that. But not in this PR, these are (all) fine for now.
There was a problem hiding this comment.
The exact error -
src\vector\backends\awkward.py:577: error: Subclass of "VectorProtocol", "Momentum", and "Vector2D" cannot exist: would have inconsistent method resolution order [unreachable]
There was a problem hiding this comment.
Okay to ignore for now, we can try to work on it later. We will probably need to think through why this is happening. Eventually.
| if issubclass(cls, Vector2D): # type: ignore[unreachable] | ||
| return "Momentum2D" # type: ignore[unreachable] | ||
| elif issubclass(cls, Vector3D): # type: ignore[unreachable] | ||
| return "Momentum3D" # type: ignore[unreachable] | ||
| elif issubclass(cls, Vector4D): # type: ignore[unreachable] | ||
| return "Momentum4D" # type: ignore[unreachable] |
There was a problem hiding this comment.
Guessing it is not understanding that Type[VectorProtocol] can (sometimes) pass a subclass of Momentum check? Might be worth updating that. But not in this PR, these are (all) fine for now.
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
setup.cfg,.pre-commit-config.yaml, andpyproject.tomlto pass the repo reviewnoxfile.pyfor beginnersRepo review for this branch - https://scikit-hep.org/repo-review/?repo=Saransh-cpp%2Fvector&branch=saransh%2Frepo-review
(The red cross should ideally not appear since the dependabot check was fixed in the repo review repository. All the other green ticks are very satisfying to look at!)