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 1 commit into
base: develop
Choose a base branch
from

Conversation

Rickisterr
Copy link

@Rickisterr Rickisterr commented May 12, 2025

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
Copilot

This comment was marked as outdated.

@Rickisterr
Copy link
Author

All required updates made to code; thank you for the reviews!

@Gui-FernandesBR Gui-FernandesBR requested a review from Copilot July 25, 2025 02:35
Copilot

This comment was marked as outdated.

@Gui-FernandesBR Gui-FernandesBR requested a review from Copilot July 25, 2025 15:10
Copilot

This comment was marked as outdated.

@Gui-FernandesBR Gui-FernandesBR requested a review from Copilot July 25, 2025 15:21
Copilot

This comment was marked as outdated.

@Gui-FernandesBR Gui-FernandesBR requested a review from Copilot July 25, 2025 15:55
Copilot

This comment was marked as outdated.

@Gui-FernandesBR Gui-FernandesBR requested a review from Copilot July 26, 2025 01:12
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

Adds crop and clip functionality to the Function class to restrict input domains and output values respectively. The crop method limits input domains by filtering data within specified ranges, while the clip method constrains output values to specified intervals.

Key changes:

  • Added crop() method to restrict function input domains
  • Added clip() method to restrict function output values
  • Modified domain handling in set_discrete() and plotting methods to respect cropped domains

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
rocketpy/mathutils/function.py Implements crop and clip methods with helper functions for domain bounds determination and source manipulation
tests/unit/test_function.py Adds comprehensive test coverage for crop and clip methods with different source types
CHANGELOG.md Documents the enhancement addition
README.md Adds DeepWiki badge
Comments suppressed due to low confidence (2)

tests/unit/test_function.py:274

  • The parameter name 'array3dsource' is misleading as it suggests a 3D array, but it's actually a 2D array with 3 columns. Consider renaming to 'array2d_source' or 'source_array'.
def test_crop_ndarray(array3dsource, array3dcropped):  # pylint: disable=unused-argument

tests/unit/test_function.py:321

  • The parameter name 'array3dsource' is misleading as it suggests a 3D array, but it's actually a 2D array with 3 columns. Consider renaming to 'array2d_source' or 'source_array'.
def test_clip_ndarray(array3dsource, array3dclipped):  # pylint: disable=unused-argument

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.

LGTM

@github-project-automation github-project-automation bot moved this from Backlog to Next Version in LibDev Roadmap Jul 26, 2025
@Gui-FernandesBR Gui-FernandesBR force-pushed the enh/function-crop-clip-methods branch 2 times, most recently from df43380 to 256d417 Compare July 26, 2025 15:09
@Gui-FernandesBR
Copy link
Member

I believe this PR is now ready for production.
However, as I did some (many) changes yesterday, I believe it would be interesting if another code owner could review the function.py file again.
@phmbressan could you take a look at it for us pls?

@Gui-FernandesBR
Copy link
Member

@codecov-ai-reviewer test

@Gui-FernandesBR Gui-FernandesBR force-pushed the enh/function-crop-clip-methods branch from 256d417 to 66b1f55 Compare July 26, 2025 21:01
Code reformatted for PyLint by Black

Updated CHANGELOG.md

Removed redundant files

Reformatted + Removed Redundanct files

Docstring Update + Code Optimization

ENH: Add Crop and Clip Methods to Function

Code reformatted for PyLint by Black

Removed redundant files

Reformatted + Removed Redundanct files

REL: update to v1.10.0

Update README.md

adds deepwiki badge

MNT: refactor Function class to use __cropped_domain__ for input limits

MNT: Refactor Function class to encapsulate code with smaller functions

Update rocketpy/mathutils/function.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update rocketpy/mathutils/function.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update rocketpy/mathutils/function.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update rocketpy/mathutils/function.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update tests/unit/test_function.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

MNT: simplify crop logic and improve code readability

BUF: fix errors on tests

Update rocketpy/mathutils/function.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update tests/unit/test_function.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

DOC? Remove plot example from clip method docstring

MNT: Fix rebasing problems

MNT: Fix rebasing problems 2
@Gui-FernandesBR Gui-FernandesBR force-pushed the enh/function-crop-clip-methods branch from 66b1f55 to 1c9a966 Compare July 26, 2025 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Next Version
Development

Successfully merging this pull request may close these issues.

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