-
-
Notifications
You must be signed in to change notification settings - Fork 142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: Function Reverse Arithmetic Priority #488
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #488 +/- ##
===========================================
+ Coverage 70.91% 71.05% +0.14%
===========================================
Files 55 55
Lines 9261 9262 +1
===========================================
+ Hits 6567 6581 +14
+ Misses 2694 2681 -13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
This will prevent me from dying a couple of years earlier due to stress. Thank you!
Why exactly is this important? I couldn't figure that out by simply reading the PR description. Does it also works for lists instead of np.ndarrays ? So are you saying this magic line array_ufunc = None is extremely important for solving the issue, is that right? |
The CHANGELOG.md needs to be updated before merging. |
Some of the modified lines were not covered by tests. Could you check/fix that please? |
To speed things up, let me try to help here. As the PR description says:
Which means, for example, This causes a lot of problems, since we expect |
@Gui-FernandesBR just made some commits expanding the tests for these operations and updated the CHANGELOG file. |
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.
LGTM,
Good use of tests
Pull request type
Checklist
black rocketpy/ tests/
) has passed locallypytest --runslow
) have passed locallyCHANGELOG.md
has been updated (if relevant)Current behavior
Operations that involve reverse
Function
operations withnumpy.ndarrays
result, usually unexpectedly, in anumpy.ndarray
as a result. This happens sincenumpy.ndarray
implement an__radd__
that overridesFunction.__radd__
operation.New behavior
The variable
__array_ufunc__
is a way to enforceFunction
priority regarding reverse operations.Breaking change
Additional information
This same issue was found during
mathutils.vector
andmathutils.matrix
development and the same fix was applied (using__array_ufunc__
).