Skip to content

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

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

Rickisterr
Copy link

Pull request type

  • Code changes (enhancement, features)

Checklist

  • Lint (black rocketpy/ tests/) has passed locally
  • All tests (pytest tests --runslow) have passed locally
  • CHANGELOG.md has been updated (if relevant)

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 and Function.crop - #546

New 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 and clip 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 variables
f.clip([(-2, 2)]) - For output variable

@Rickisterr Rickisterr requested a review from a team as a code owner May 12, 2025 17:06
@Rickisterr Rickisterr changed the base branch from master to develop May 12, 2025 17:09
@Gui-FernandesBR
Copy link
Member

WE no longer use black, can you please run make format so we can review the PR faster?

@Rickisterr
Copy link
Author

All required changes have been updated. Thank you!

Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a 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

@Gui-FernandesBR Gui-FernandesBR linked an issue May 16, 2025 that may be closed by this pull request
2 tasks
@Gui-FernandesBR Gui-FernandesBR force-pushed the enh/function-crop-clip-methods branch from 92bc774 to 72d6f5d Compare May 16, 2025 17:36
@Gui-FernandesBR Gui-FernandesBR requested a review from Copilot May 16, 2025 21:05
@Gui-FernandesBR Gui-FernandesBR force-pushed the enh/function-crop-clip-methods branch from 72d6f5d to f73c336 Compare May 16, 2025 21:05
Copy link

@Copilot Copilot AI left a 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)])
Copy link
Preview

Copilot AI May 16, 2025

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.

Comment on lines +1035 to +1046
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)))
Copy link
Preview

Copilot AI May 16, 2025

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.

Suggested change
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

ENH: give me a scissor or apply Function.clip and Function.crop
2 participants