Skip to content
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

Preserve dtypes in XRImage "enhancements" #151

Merged
merged 26 commits into from
Nov 22, 2023

Conversation

pnuu
Copy link
Member

@pnuu pnuu commented Nov 17, 2023

Some of the "enhancments" in XRImage class currently upcasts float data to float64. This PR makes these (.gamma() and .colorize() to preserve the dtype of the input data. Checks are added also to other tests to check this holds true also for those operations.

Note that #150 has the same done for .crude_stretch().

Includes also some test refactoring.

  • Closes #xxxx (remove if there is no corresponding issue, which should only be the case for minor changes)
  • Tests added (for all bug fixes or enhancements)
  • Tests passed (for all non-documentation changes)
  • Passes git diff origin/master **/*py | flake8 --diff (remove if you did not edit any Python files)
  • Fully documented (remove if this change should not be visible to users, e.g., if it is an internal clean-up, or if this is part of a larger project that will be documented later)

Copy link

codecov bot commented Nov 17, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (9564cba) 91.28% compared to head (76b52aa) 91.47%.

Files Patch % Lines
trollimage/xrimage.py 95.65% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #151      +/-   ##
==========================================
+ Coverage   91.28%   91.47%   +0.18%     
==========================================
  Files          11       11              
  Lines        3890     3952      +62     
==========================================
+ Hits         3551     3615      +64     
+ Misses        339      337       -2     
Flag Coverage Δ
unittests 91.47% <99.14%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@coveralls
Copy link

coveralls commented Nov 17, 2023

Coverage Status

coverage: 91.713% (+0.2%) from 91.529%
when pulling 76b52aa on pnuu:preserve-enhancement-dtypes
into 9564cba on pytroll:main.

trollimage/xrimage.py Outdated Show resolved Hide resolved
trollimage/xrimage.py Outdated Show resolved Hide resolved
trollimage/xrimage.py Outdated Show resolved Hide resolved
trollimage/xrimage.py Outdated Show resolved Hide resolved
Co-authored-by: David Hoese <david.hoese@ssec.wisc.edu>
Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, some comments

trollimage/tests/test_image.py Outdated Show resolved Hide resolved
trollimage/xrimage.py Show resolved Hide resolved
@@ -1954,7 +1986,8 @@ def test_blend(self):
[0.768815, 0.72, 0.6728228, 0.62857145, 0.5885714],
[0.55412847, 0.5264665, 0.50666666, 0.495612, 0.49394494],
[0.5020408, 0.52, 0.5476586, 0.5846154, 0.63027024],
[0.683871, 0.7445614, 0.81142855, 0.8835443, 0.96]]))
[0.683871, 0.7445614, 0.81142855, 0.8835443, 0.96]], dtype=np.float32),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it looks like all the tests above this point were converted to float32, and the regular float version was just dropped. Maybe it's not necessary for all tests, but could dtype maybe be parameterized so the we don't just forget about floats here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the parametrize decorator to this and other tests to test with np.float32, np.float64 and float.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, thanks!

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with this, great work!

@mraspaud mraspaud merged commit 69793ca into pytroll:main Nov 22, 2023
25 checks passed
@pnuu pnuu deleted the preserve-enhancement-dtypes branch November 22, 2023 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants