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

Add Vaisala GLD360-reader. #687

Merged
merged 26 commits into from Jun 4, 2019
Merged

Add Vaisala GLD360-reader. #687

merged 26 commits into from Jun 4, 2019

Conversation

sjoro
Copy link
Collaborator

@sjoro sjoro commented Apr 1, 2019

Add a draft-version reader for Vaisala GLD360 (Global Lightning Detection) data.

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

@sjoro
Copy link
Collaborator Author

sjoro commented Apr 1, 2019

Reader done with test data representing one full day, supported file type is flashes_20170620.txt. This is not the official file type from GLD360 network.

@mraspaud mraspaud added enhancement code enhancements, features, improvements component:readers labels Apr 2, 2019
@sjoro sjoro marked this pull request as ready for review April 5, 2019 09:23
@codecov
Copy link

codecov bot commented Apr 5, 2019

Codecov Report

Merging #687 into master will increase coverage by 0.04%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #687      +/-   ##
==========================================
+ Coverage   82.65%   82.69%   +0.04%     
==========================================
  Files         161      163       +2     
  Lines       23435    23491      +56     
==========================================
+ Hits        19370    19426      +56     
  Misses       4065     4065
Impacted Files Coverage Δ
satpy/tests/reader_tests/__init__.py 98% <100%> (+0.04%) ⬆️
satpy/readers/vaisala_gld360.py 90% <90%> (ø)
satpy/tests/reader_tests/test_vaisala_gld360.py 96% <96%> (ø)
satpy/readers/__init__.py 94.87% <0%> (+1.2%) ⬆️

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 af826ff...793899d. Read the comment docs.

@coveralls
Copy link

coveralls commented Apr 5, 2019

Coverage Status

Coverage increased (+0.05%) to 82.696% when pulling 793899d on sjoro:gld360-reader into af826ff on pytroll:master.

@mraspaud
Copy link
Member

mraspaud commented May 7, 2019

@sjoro What's the status on this ? I the PR feature-complete ? do you plan to add tests ?

@sjoro
Copy link
Collaborator Author

sjoro commented May 7, 2019

@mraspaud from my side it done for now. was thinking about possible test(s) for checking the sensitivity for the input format, but i'd like to get some brainstorming help with that.

@mraspaud
Copy link
Member

mraspaud commented May 7, 2019

Ok, we can talk about it next week.

@sjoro sjoro requested a review from sfinkens as a code owner May 13, 2019 19:05
@sjoro
Copy link
Collaborator Author

sjoro commented May 13, 2019

Added a test for the file handler.

@pnuu pnuu added the PCW Pytroll Contributors' Week label May 13, 2019
@sjoro sjoro removed this from In progress in Pytroll Contribution Week at Eumetsat May 13, 2019
@pnuu pnuu added this to In progress in Pytroll Contributor's Week - Spring 2019 - Madison, WI via automation May 13, 2019
@pnuu pnuu moved this from In progress to Needs review in Pytroll Contributor's Week - Spring 2019 - Madison, WI May 13, 2019
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! One general idea: What about adding time, longitude and latitude as non-dimensional y-coordinates to the power dataset?

xarr = xr.DataArray(da.from_array(self.data['power'],
                                  chunks=CHUNK_SIZE), dims=["y"])
xarr['time'] = ('y', self.data['time'])
xarr['longitude'] = ('y', self.data['longitude'])
xarr['latitude'] = ('y', self.data['latitude'])

You would have to set units and standard name manually though.

satpy/readers/vaisala_gld360.py Outdated Show resolved Hide resolved
satpy/readers/vaisala_gld360.py Outdated Show resolved Hide resolved
satpy/etc/readers/vaisala_gld360.yaml Show resolved Hide resolved
satpy/tests/reader_tests/test_vaisala_gld360.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_vaisala_gld360.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_vaisala_gld360.py Outdated Show resolved Hide resolved
satpy/tests/reader_tests/test_vaisala_gld360.py Outdated Show resolved Hide resolved
@sjoro sjoro requested a review from sfinkens May 16, 2019 21:33
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.

Looks good, just some nitpicking.

satpy/readers/vaisala_gld360.py Outdated Show resolved Hide resolved
satpy/etc/readers/vaisala_gld360.yaml Show resolved Hide resolved
Pytroll Contributor's Week - Spring 2019 - Madison, WI automation moved this from Needs review to Reviewer approved May 17, 2019
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.

Another nitpicker... 😄 Otherwise LGTM!

satpy/etc/readers/vaisala_gld360.yaml Outdated Show resolved Hide resolved
satpy/etc/readers/vaisala_gld360.yaml Outdated Show resolved Hide resolved
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.

LGTM

@mraspaud mraspaud added this to the v0.16 milestone May 18, 2019
@mraspaud
Copy link
Member

@sjoro can you add this reader to the reader table in the index.rst file (in the doc) ?

@mraspaud mraspaud merged commit 670fa4c into pytroll:master Jun 4, 2019
Pytroll Contributor's Week - Spring 2019 - Madison, WI automation moved this from Reviewer approved to Done Jun 4, 2019
@sjoro sjoro deleted the gld360-reader branch June 4, 2019 11:08
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 PCW Pytroll Contributors' Week
Development

Successfully merging this pull request may close these issues.

None yet

6 participants