-
Notifications
You must be signed in to change notification settings - Fork 18
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
Crop edges update simplified #177
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportBase: 100.00% // Head: 100.00% // No change to project coverage 👍
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #177 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 12 12
Lines 545 614 +69
=========================================
+ Hits 545 614 +69
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
As discussed, this is now ready for review @jlaehne . Hope it is easier to see what is new. |
@@ -23,47 +23,18 @@ | |||
|
|||
from numpy import isnan | |||
from warnings import warn | |||
from lumispy.utils.signals import crop_edges | |||
|
|||
|
|||
class CommonLumi: | |||
"""**General luminescence signal class (dimensionless)**""" | |||
|
|||
def crop_edges(self, crop_px): |
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.
To make our work for the 1.0 release easier, add a comment in the line before:
# Deprecated, to be removed for v1.0 release
@@ -23,18 +23,12 @@ | |||
|
|||
|
|||
class TestCommonLumi: | |||
def test_crop_edges(self): | |||
def test_crop_edges_deprecated(self): |
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.
add comment before # Deprecated, to be removed for v1.0 release
>>> signals_cropped = lum.utils.crop_edges(signals, crop_range=5, crop_units="%", rebin_nav=True) | ||
>>> signals_cropped | ||
[CLSpectrum <243,243|1024>, Signal2D <243,243|1>] | ||
.. Note:: |
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.
Add blank line before
Cropping along the navigation axes of a list of signal objects. | ||
Crop the amount of pixels from the four edges of the scanning | ||
region, from the edges inwards. Cropping can happen uniformly on all | ||
sides or by specifying the cropping range for each axis or each side. If the navigation axes shape is different, all signals can be rebinned to match the shape of the first signal in the list. |
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.
Continue with line breaks for readability across the whole docstring and also in error messages below.
sides or by specifying the cropping range for each axis or each side. If the navigation axes shape is different, all signals can be rebinned to match the shape of the first signal in the list. | ||
Parameters | ||
---------- | ||
S : list of HyperSpy Signal objects or a single HyperSpy Signal object. |
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.
S : list of HyperSpy Signal objects or a single HyperSpy Signal object. | |
S : Signal or list of Signals | |
HyperSpy signal object(s) that should be cropped. |
If the navigation axes shape is different between signals in the list S, all signals will be rebinned to match the shape of the first signal in the list. Note this does not take into account the calibration values of the navigation axes. | ||
kwrgs | ||
To account for the deprecated ``crop_px`` parameter. | ||
Returns |
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.
Returns | |
Returns |
Description of the change
Following the request to make #142 cleaner and separate the
crop_ede
function from other changes, this is the clean version of the PR, aiming to extend the functionality ofcrop_edges
.Progress of the PR