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

Change logic for ninjogeotiff gradient/axisintercept tags #2491

Merged
merged 3 commits into from
May 25, 2023

Conversation

gerritholl
Copy link
Collaborator

In the ninjogeotiff writer, change the logic for deciding whether to set the tags AxisIntercept and Gradient. Previously, those were always set, unless the image had mode RGB or RGBA. With this change, those tags are set only if the image has mode L and LA and PhysicUnit is not set to N/A.

This commit also makes a small bugfix in a data fixture in the test routine, that leads to a few reference values changing slightly. This reflects no change in functionality, but only between the fixtures and the test functions.

Adapt documentation to reflect that NinJo 7 is now out and about.

In the ninjogeotiff writer, change the logic for deciding whether or
not to set the tags AxisIntercept and Gradient.  Previously, those were
always set, unless the image had mode RGB or RGBA.  With this change,
those tags are set only if the image has mode L and LA and PhysicUnit is
not set to N/A.

This commit also makes a small bugfix in a data fixture in the test
routine, that leads to a few reference values changing slightly.  This
reflects no change in functionality, but only between the fixtures and
the test functions.

Adapt documentation to reflect that NinJo 7 is now out and about.
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 think this looks good, even though I'm not familiar with the format. Just one question inline.

satpy/writers/ninjogeotiff.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Merging #2491 (feaf9cc) into main (3b7a477) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2491   +/-   ##
=======================================
  Coverage   94.83%   94.83%           
=======================================
  Files         337      337           
  Lines       49430    49451   +21     
=======================================
+ Hits        46875    46896   +21     
  Misses       2555     2555           
Flag Coverage Δ
behaviourtests 4.43% <0.00%> (-0.01%) ⬇️
unittests 95.45% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
satpy/tests/writer_tests/test_ninjogeotiff.py 96.66% <100.00%> (+0.14%) ⬆️
satpy/writers/ninjogeotiff.py 99.30% <100.00%> (+0.02%) ⬆️

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5076940443

  • 27 of 27 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 95.401%

Totals Coverage Status
Change from base Build 5075221574: 0.002%
Covered Lines: 47009
Relevant Lines: 49275

💛 - Coveralls

@gerritholl
Copy link
Collaborator Author

CodeScene is wrong or unreasonable. It complains my method has increased to 92 lines of code, but I count only 33 physical or 13 logical lines of code (unless we include the docstring, which can't be right). The number of logical lines of code stays the same in this PR, and I have actually factored out some functionality. I do not agree with CodeScenes conclusion that the code health has decreased.

@mraspaud mraspaud merged commit 3fd4c45 into pytroll:main May 25, 2023
@mraspaud
Copy link
Member

code health is good enough for me, merged.

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

Successfully merging this pull request may close these issues.

ninjogeotiff writer adds offset/scale factor when this is not meaningful
4 participants