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

Various optimizations to colorspace code #143

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Sep 17, 2023

I did a few little ones to allow a clearer path for the compiler to recognized clipping a value in one of the conversions to 0.0 and 1.0. The bigger update I did to have everything use a single memory view as input and output hopefully saves memory...maybe, but it doesn't seem to improve execution time which is surprising and extremely disappointing.

  • 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)

@djhoese djhoese self-assigned this Sep 17, 2023
@djhoese djhoese marked this pull request as draft September 17, 2023 02:00
Also convert C constants to "#define" macros
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Merging #143 (7d256a4) into main (06e2ae6) will increase coverage by 0.04%.
The diff coverage is 71.87%.

@@            Coverage Diff             @@
##             main     #143      +/-   ##
==========================================
+ Coverage   91.18%   91.23%   +0.04%     
==========================================
  Files          11       11              
  Lines        3847     3832      -15     
==========================================
- Hits         3508     3496      -12     
+ Misses        339      336       -3     
Flag Coverage Δ
unittests 91.23% <71.87%> (+0.04%) ⬆️

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

Files Changed Coverage Δ
trollimage/tests/test_image.py 100.00% <ø> (ø)
trollimage/_colorspaces.pyx 61.90% <68.14%> (-2.36%) ⬇️
trollimage/colormap.py 100.00% <100.00%> (ø)
trollimage/tests/test_colormap.py 100.00% <100.00%> (ø)

@coveralls
Copy link

coveralls commented Sep 18, 2023

Coverage Status

coverage: 91.479% (+0.04%) from 91.435% when pulling 7d256a4 on djhoese:opt-colorspaces into 06e2ae6 on pytroll:main.

@djhoese
Copy link
Member Author

djhoese commented Nov 22, 2023

@pnuu Looks like I was playing around with some dtype preservation in this PR too (specifically in colorize). I'll need to review this next week.

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.

2 participants