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 handling of file attributes in seviri_l1b_nc reader #791

Merged
merged 2 commits into from
May 28, 2019

Conversation

ColinDuff
Copy link
Contributor

EUMETSAT have corrected the netcdf attributes for the combined VIS/IR/HRV Level1b netcdf product.
This PR removes the previous workaround and also reads/displays the HRV data correctly
Closes #753

Test for netcdf Level1b data is WIP - as the reader is currently available i thought best to fix it first then add the reader later

No issues raised with flake8

@mraspaud
Copy link
Member

@ColinDuff that's great, thanks ! Will this work also with older files from the archive ?

@coveralls
Copy link

coveralls commented May 23, 2019

Coverage Status

Coverage decreased (-0.01%) to 82.268% when pulling 0188475 on ColinDuff:seviri_netcdf_fix into cd65262 on pytroll:master.

@codecov
Copy link

codecov bot commented May 23, 2019

Codecov Report

Merging #791 into master will decrease coverage by <.01%.
The diff coverage is 5.4%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #791      +/-   ##
==========================================
- Coverage   82.27%   82.27%   -0.01%     
==========================================
  Files         159      159              
  Lines       22996    22998       +2     
==========================================
  Hits        18921    18921              
- Misses       4075     4077       +2
Impacted Files Coverage Δ
satpy/readers/seviri_l1b_nc.py 15.53% <5.4%> (-2.29%) ⬇️
satpy/scene.py 90.08% <0%> (+0.17%) ⬆️
satpy/readers/avhrr_l1b_gaclac.py 91.42% <0%> (+1.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd65262...0188475. Read the comment docs.

@ColinDuff
Copy link
Contributor Author

Hi Martin - no it will not work with older files that users may have ordered and are stored locally.

I assume any netcdf level1b file ordered from EUM now will have the correct attributes . So they may have to reorder any files they have issues with.

@ColinDuff
Copy link
Contributor Author

In response to my assumption re older seviri Level1b netcdf files I tested succesfully on 2 seviri level1b netcdf files ordered from 2017.

Copy link
Member

@sfinkens sfinkens left a comment

Choose a reason for hiding this comment

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

Nice work! Just two comments, otherwise looks good to me. If you have time, it would be nice if you could add some tests for the reader :)

if earth_model == 2:
ns_offset = 0 # north +ve
we_offset = 0 # west +ve
elif earth_model == 1:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a short comment that the data is shifted to N-W and to correct for it, we have to adjust the corners in the opposite direction, i.e. S-E

Copy link
Collaborator

Choose a reason for hiding this comment

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

the comment can be copied from the seviri_l1b_native-reader.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Working on creating tests for this reader now

I will add the comment as suggested by sjoro

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment added

file_reader: !!python/name:satpy.readers.seviri_l1b_nc.NCSEVIRIFileHandler
file_patterns: ['W_XX-EUMETSAT-Darmstadt,VIS+IR+IMAGERY,{satid:4s}+SEVIRI_C_EUMG_{processing_time:%Y%m%d%H%M%S}.nc']
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the VIS+IR+IMAGERY pattern still needed if one de-selects HRV in the order (EUM data centre)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hi, when i ordered the data the Netcdf option didnt allow me to deselect any channels. But now I see that the Netcdf (GSICS) option does so i will update the yaml and test this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so there is an issue with the netcdf data ordered using netcdf (GSICS) as some attributes are missing, Also spacecraft id is missing from the filename. EUM has been informed

Copy link
Member

Choose a reason for hiding this comment

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

Oh, too bad. Shall we wait for them to fix it or merge now and add support for GSICS nc-files later?

Copy link
Collaborator

Choose a reason for hiding this comment

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

after talking to @ColinDuff, I suggest we merge this now and deal with GSICS nc-files later. this way users can use the reader for "regular" nc-files.

@mraspaud
Copy link
Member

Thanks for checking this out.

Having an open issue for the tests would be nice. Otherwise I can merge this when we get a green light from @sfinkens (and maybe @sjoro if he wants to participate).

Copy link
Collaborator

@sjoro sjoro left a comment

Choose a reason for hiding this comment

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

if i remember correctly, this reader works for certain file_patterns, but not all? some netCDF-files do not contain all the relevant info for the reader, right? if this is the case, please add all possible file_patterns (VIS+IR, HRV, VIS+IR+HRV) to the reader yaml-file. raise a NotImplemented error for the ones the reader does not work.

@djhoese djhoese changed the title Fixes Issue 753 - handles nc file attrubutes correctly and reads/disp… Fix handling of file attributes in seviri_l1b_nc reader May 28, 2019
@djhoese djhoese merged commit 6f1f451 into pytroll:master May 28, 2019
@ColinDuff ColinDuff deleted the seviri_netcdf_fix branch November 11, 2020 08:17
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.

seviri l1b netcdf reader needs to be updated due to EUM fixing Attribute Issue
6 participants