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.

Sign up for GitHub

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

Jump to bottom

Daskify and test avhrr_l1b_aapp reader #811

Merged
merged 25 commits into from Jan 30, 2020
Merged

Daskify and test avhrr_l1b_aapp reader #811

merged 25 commits into from Jan 30, 2020

Conversation

mraspaud
Copy link
Member

@mraspaud mraspaud commented Jun 10, 2019 •
edited

This PR daskifies and tests the avhrr_l1b_aapp reader

Informal tests show a speedup from 7.9s to 5.5s to generate a composite on my laptop.

  • Closes Update avhrr_l1b_aapp reader #809
  • Tests added and test suite added to parent suite
  • Tests passed
  • Passes flake8 satpy
  • Fully documented
  • Add your name to AUTHORS.md if not there already

@mraspaud mraspaud added enhancement code enhancements, features, improvements help wanted component:readers labels Jun 10, 2019
@mraspaud mraspaud self-assigned this Jun 10, 2019
@coveralls
Copy link

coveralls commented Jan 8, 2020 •
edited

Coverage Status

Coverage increased (+0.5%) to 88.073% when pulling 9543b93 on mraspaud:fix-avhrr-reader into 17efb75 on pytroll:master.

@codecov
Copy link

codecov bot commented Jan 8, 2020 •
edited

Codecov Report

Merging #811 into master will increase coverage by 0.04%.
The diff coverage is 97.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #811      +/-   ##
==========================================
+ Coverage   88.03%   88.08%   +0.04%     
==========================================
  Files         189      191       +2     
  Lines       29323    29412      +89     
==========================================
+ Hits        25815    25908      +93     
+ Misses       3508     3504       -4
Impacted Files Coverage Δ
satpy/tests/reader_tests/test_ami_l1b.py 96.61% <100%> (+1.82%) ⬆️
satpy/tests/reader_tests/__init__.py 98.46% <100%> (+0.02%) ⬆️
satpy/config.py 82.14% <100%> (+1.27%) ⬆️
satpy/readers/hrpt.py 6.01% <100%> (+2.16%) ⬆️
satpy/tests/test_yaml_reader.py 99.57% <100%> (+0.43%) ⬆️
satpy/readers/yaml_reader.py 94.42% <100%> (+0.02%) ⬆️
satpy/readers/slstr_l2.py 93.54% <93.54%> (ø)
satpy/tests/reader_tests/test_slstr_l2.py 97.77% <97.77%> (ø)
... and 1 more

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 f41918a...9543b93. Read the comment docs.

@mraspaud mraspaud marked this pull request as ready for review January 9, 2020 10:03
@mraspaud
Copy link
Member Author

@pnuu @adybbroe @djhoese does anyone want to review this or can I just merge ?

djhoese
djhoese approved these changes Jan 29, 2020
View reviewed changes
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.

One small comment but otherwise looks good to me. It is too bad that we can't do the checks for particular channels, but eh that's ok.

satpy/readers/aapp_l1b.py Show resolved Hide resolved
pnuu
pnuu approved these changes Jan 30, 2020
View reviewed changes
Copy link
Member

@pnuu pnuu left a comment

Choose a reason for hiding this comment

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

LGTM. Tested with few scenes and composites, no issues found.

@mraspaud
Copy link
Member Author

@djhoese @pnuu thanks for taking the time to review! Merging now.

@mraspaud mraspaud merged commit e25415b into pytroll:master Jan 30, 2020
@mraspaud mraspaud deleted the fix-avhrr-reader branch January 30, 2020 07:24
@djhoese djhoese changed the title Daskify and test avhrr_l1b_aapp reader Daskify and test avhrr_l1b_aapp reader Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djhoese djhoese djhoese approved these changes

@pnuu pnuu pnuu approved these changes

@adybbroe adybbroe Awaiting requested review from adybbroe

Assignees

@mraspaud mraspaud

Labels
component:readers enhancement code enhancements, features, improvements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update avhrr_l1b_aapp reader
4 participants
@mraspaud @coveralls @djhoese @pnuu