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

Support CLASS packed viirs files in viirs_sdr reader #538

Merged
merged 20 commits into from Mar 15, 2019

Conversation

mraspaud
Copy link
Member

@mraspaud mraspaud commented Dec 7, 2018

This PR adds support for viirs sdr files containing more than one band/dataset at the time.

  • Tests added
  • Tests passed
  • Passes git diff origin/master -- "*py" | flake8 --diff
  • Fully documented

@mraspaud mraspaud added enhancement code enhancements, features, improvements component:readers work in progress labels Dec 7, 2018
@mraspaud mraspaud self-assigned this Dec 7, 2018
@mraspaud
Copy link
Member Author

mraspaud commented Dec 7, 2018

What is left is find a way to prioritize TC geo files.

file_key: 'All_Data/{file_group}_All/SolarZenithAngle'
file_type: generic_file
dataset_groups: [GDNBO]
file_key: 'All_Data/{dataset_group}_All/SolarZenithAngle'
DNB_LZA:
name: dnb_lunar_zenith_angle
standard_name: lunar_zenith_angle
resolution: 743
coordinates: [dnb_longitude, dnb_latitude]
file_type: gdnbo
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be file_type: generic_file to?

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

get_bounding_box needs to be handled too, right? And we'll have to figure out how to handle the custom Reader at the bottom of the module which let's the user force TC or not.

Conflicts:
	satpy/etc/readers/viirs_sdr.yaml
@djhoese
Copy link
Member

djhoese commented Mar 11, 2019

Any idea if this PR works with old DNB files that didn't have TC data?

@mraspaud
Copy link
Member Author

use_tc can be either True, False or None, with None being 'use TC if available, non-TC otherwise'.

@mraspaud
Copy link
Member Author

This PR will work with old DNB file as long as use_tc isn't True

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 78.124% when pulling 4bf7b8d on mraspaud:feature-viirs-class into 0cefc09 on pytroll:master.

@coveralls
Copy link

coveralls commented Mar 11, 2019

Coverage Status

Coverage increased (+0.09%) to 78.466% when pulling fd77a96 on mraspaud:feature-viirs-class into fb39c41 on pytroll:master.

@codecov
Copy link

codecov bot commented Mar 11, 2019

Codecov Report

Merging #538 into master will increase coverage by 0.07%.
The diff coverage is 86.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #538      +/-   ##
==========================================
+ Coverage   78.39%   78.46%   +0.07%     
==========================================
  Files         138      138              
  Lines       20013    20252     +239     
==========================================
+ Hits        15689    15891     +202     
- Misses       4324     4361      +37
Impacted Files Coverage Δ
satpy/tests/reader_tests/test_ahi_hsd.py 96.51% <ø> (ø) ⬆️
satpy/multiscene.py 87.08% <ø> (+0.53%) ⬆️
satpy/tests/reader_tests/test_goes_imager_nc.py 98.13% <ø> (-0.01%) ⬇️
satpy/writers/geotiff.py 42.01% <0%> (ø) ⬆️
satpy/composites/viirs.py 83.71% <100%> (ø) ⬆️
satpy/readers/yaml_reader.py 92.3% <100%> (+0.04%) ⬆️
satpy/tests/reader_tests/test_viirs_sdr.py 92.28% <84.86%> (-6.43%) ⬇️
satpy/readers/viirs_sdr.py 84.75% <88.33%> (+3.82%) ⬆️
satpy/readers/nwcsaf_nc.py 35.18% <0%> (-0.67%) ⬇️

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 fb39c41...fd77a96. Read the comment docs.

@mraspaud mraspaud requested a review from djhoese March 11, 2019 23:18
@mraspaud mraspaud added this to the v0.13 milestone Mar 12, 2019
@mraspaud
Copy link
Member Author

@djhoese any more change requests on this PR ?

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

Nice job some how cleaning up the code while also adding more functionality. I had a couple small docstring things. Otherwise, you obviously tested this with files where multiple datasets were combined in to one file, but what about multiple times in one file (multiple granules)?

satpy/composites/viirs.py Show resolved Hide resolved
satpy/writers/geotiff.py Show resolved Hide resolved
satpy/readers/viirs_sdr.py Show resolved Hide resolved
satpy/readers/viirs_sdr.py Outdated Show resolved Hide resolved
satpy/readers/viirs_sdr.py Outdated Show resolved Hide resolved
@mraspaud
Copy link
Member Author

Regarding multiple times in one file (multiple granules), I test this with the bounding box test.

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

LGTM

@djhoese djhoese merged commit be6e5a0 into pytroll:master Mar 15, 2019
@mraspaud mraspaud deleted the feature-viirs-class branch March 15, 2019 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:readers enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants