-
-
Notifications
You must be signed in to change notification settings - Fork 189
ENH: Added Crop and Clip Methods to Function Class #817
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
base: develop
Are you sure you want to change the base?
ENH: Added Crop and Clip Methods to Function Class #817
Conversation
WE no longer use black, can you please run |
All required changes have been updated. Thank you! |
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.
Great PR! We will review this implementation as soon as possible
92bc774
to
72d6f5d
Compare
72d6f5d
to
f73c336
Compare
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.
Pull Request Overview
This PR introduces new crop and clip methods to the Function class, enabling users to restrict input and output ranges of functions. Key changes include new test cases for 1-D and 2-D functions, modifications in the Function class to implement cropping and clipping behaviors, and an updated CHANGELOG entry.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
tests/unit/test_function.py | Added tests for crop and clip methods with various input scenarios, including handling None values for unconstrained axes. |
rocketpy/mathutils/function.py | Implemented crop and clip methods, updating set_discrete and interpolation boundaries accordingly. |
CHANGELOG.md | Added an entry documenting the enhancement. |
>>> f2 = Function(lambda x1, x2: np.cos(x1)*np.sin(x2), inputs=['x1', 'x2'], outputs='y') | ||
>>> f2 | ||
Function from R2 to R1 : (x1, x2) → (y) | ||
>>> f2.crop([None, (-2, 2)]) |
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.
Consider updating the crop() docstring to clarify that passing None for a dimension means no cropping constraint on that axis, as illustrated by the example.
Copilot uses AI. Check for mistakes.
elif callable(self.source): | ||
if isinstance(self.source, NUMERICAL_TYPES): | ||
try: | ||
if self.source < y_lim[0][0]: | ||
raise ArithmeticError("Constant function outside range") | ||
if self.source > y_lim[0][1]: | ||
raise ArithmeticError("Constant function outside range") | ||
except TypeError as e: | ||
raise TypeError("y_lim must be same type as function source") from e | ||
else: | ||
f = self.source | ||
self.source = lambda x: max(y_lim[0][0], min(y_lim[0][1], f(x))) |
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.
[nitpick] Review the constant-function logic in the clip() method; the branch checking for NUMERICAL_TYPES on a callable source may be ambiguous. Consider refactoring this logic to clearly distinguish constant functions from other callables.
elif callable(self.source): | |
if isinstance(self.source, NUMERICAL_TYPES): | |
try: | |
if self.source < y_lim[0][0]: | |
raise ArithmeticError("Constant function outside range") | |
if self.source > y_lim[0][1]: | |
raise ArithmeticError("Constant function outside range") | |
except TypeError as e: | |
raise TypeError("y_lim must be same type as function source") from e | |
else: | |
f = self.source | |
self.source = lambda x: max(y_lim[0][0], min(y_lim[0][1], f(x))) | |
elif isinstance(self.source, NUMERICAL_TYPES): | |
try: | |
if self.source < y_lim[0][0]: | |
raise ArithmeticError("Constant function outside range") | |
if self.source > y_lim[0][1]: | |
raise ArithmeticError("Constant function outside range") | |
except TypeError as e: | |
raise TypeError("y_lim must be the same type as the function source") from e | |
elif callable(self.source): | |
f = self.source | |
self.source = lambda x: max(y_lim[0][0], min(y_lim[0][1], f(x))) |
Copilot uses AI. Check for mistakes.
Pull request type
Checklist
Current behavior
RocketPy Function class has no methods to crop or clip the input and output values of a function to a specific range.
This PR is related to the following issue:
ENH: give me a scissor or apply
Function.clip
andFunction.crop
- #546New behavior
It is now possible to specify a limited range of values for input or output variables for an object created from the Function class at any point after initialization of the object by using the
crop
andclip
methods respectively. These methods work for any type of allowed Function source.Breaking change
No
Additional information
To use the new methods, some examples are provided below to exemplify their syntax in brief:
f.crop([(-1, 1), (-2, 2)])
- For two input variablesf.clip([(-2, 2)])
- For output variable