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

Fix missing radiance units in eps l1b #2655

Merged
merged 1 commit into from
Dec 1, 2023

Conversation

mraspaud
Copy link
Member

This PR fixes a bug preventing reading radiance from avhrr eps l1b data.

Copy link

codecov bot commented Nov 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e338294) 95.29% compared to head (9c9ae08) 95.29%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2655   +/-   ##
=======================================
  Coverage   95.29%   95.29%           
=======================================
  Files         369      369           
  Lines       52040    52049    +9     
=======================================
+ Hits        49591    49600    +9     
  Misses       2449     2449           
Flag Coverage Δ
behaviourtests 4.19% <0.00%> (-0.01%) ⬇️
unittests 95.91% <100.00%> (+<0.01%) ⬆️

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.

@@ -144,7 +144,8 @@ class EPSAVHRRFile(BaseFileHandler):
sensors = {"AVHR": "avhrr-3"}

units = {"reflectance": "%",
"brightness_temperature": "K"}
"brightness_temperature": "K",
Copy link
Member

Choose a reason for hiding this comment

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

Just because I feel like being a pain today...if this had ended with a comma and the } was on its own line, it wouldn't have been part of the diff. The history...the git history!!! 😱

Copy link
Member Author

Choose a reason for hiding this comment

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

But then again, that would require from us to read one more line to understand the dictionary is closed here. Also github could be smarter and just show the actual diffing chars, like eg delta does to make reviews easier.
https://github.com/dandavison/delta

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes syntax diffs are nice. In the "read one more line" argument, why do you need to know the dictionary is closed (ignoring that it is just one more line)? You are going to keep reading the code to see where the dictionary is used anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

black would reformat it like that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's why we don't have it 😀

Copy link
Member

Choose a reason for hiding this comment

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

Lol yeah sorry, there is a much longer history to this discussion that isn't obvious from my single comment here.

@mraspaud mraspaud merged commit 25786b6 into pytroll:main Dec 1, 2023
17 of 19 checks passed
@mraspaud mraspaud deleted the fix-eps-radiance branch December 1, 2023 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to read radiance with AVHRR EPS
3 participants