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 wradlib #723

Merged
merged 2 commits into from
Apr 3, 2018
Merged

Fix wradlib #723

merged 2 commits into from
Apr 3, 2018

Conversation

kmuehlbauer
Copy link
Contributor

@kmuehlbauer kmuehlbauer commented Apr 3, 2018

@scollis
Copy link
Member

scollis commented Apr 3, 2018

Thanks @kmuehlbauer !! @zssherman can you review.. Download the branch and make sure we can still read X-SAPR2

@scollis scollis requested a review from zssherman April 3, 2018 13:50
@zssherman
Copy link
Collaborator

Sounds good, doing that now.

@zssherman
Copy link
Collaborator

@scollis Sorry for clarification, X-SAPR2 is gamic, did you still want me to test that or did you have a rainbow file I should test too?

@kmuehlbauer
Copy link
Contributor Author

@zssherman only rainbow is effected.

@zssherman
Copy link
Collaborator

zssherman commented Apr 3, 2018

@kmuehlbauer Makes sense. Ah I notice that link changes, I changed the wradlib link in the README, didn't realize there were more. Glad you caught those. @scollis The changes look good to me and the CI is passing, I just need some rainbow data if you want me to be completely sure, otherwise I see no issues.

@kmuehlbauer
Copy link
Contributor Author

@zssherman there is one rainbow file in wradlib/wradlib-data repo on github, if you want to test.

@kmuehlbauer
Copy link
Contributor Author

@zssherman Only the function name changed, no functionality. Should work.

@zssherman
Copy link
Collaborator

Perfect! Thanks @kmuehlbauer !

@scollis
Copy link
Member

scollis commented Apr 3, 2018

Ahh cool.. for some reason I thought GAMIC had rainbow as an underlying format.. @zssherman just double check using the rainbow file..

Copy link
Collaborator

@zssherman zssherman 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, I tested it with wradlib=1.0.0 and wradlib-0.11.3 and it worked for both.

@scollis
Copy link
Member

scollis commented Apr 3, 2018

Thanks! accepting

@scollis scollis merged commit 3b2dc49 into ARM-DOE:master Apr 3, 2018
@meteoswiss-mdr
Copy link
Contributor

Hi all,

@kmuehlbauer , @scollis, there was an issue with the wradlib library. When passing the file name instead of the file handle the file was not properly closed after reading. That issue has been corrected but it did not make it to version 1.0 of wradlib so whoever uses wradlib 1.0 in Py-ART (as myself) will not close the files properly
I propose to solve the issue by passing the file handle, not the file name to wradlib, i.e.:
with open(filename, 'rb') as fid:
rbf = read_rainbow(fid, loaddata=True)
Greetings from Switzerland!

@scollis
Copy link
Member

scollis commented Aug 22, 2018

Good morning @meteoswiss-mdr ! Sounds like a good change. Feel free to submit a PR or we can do so next week (DoE meeting this week).

@meteoswiss-mdr
Copy link
Contributor

It is such a small change that I let you do it yourselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wradlib release 1.0.0
4 participants