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

download function for JAXA's AW3D30 DEM #734

Merged
merged 16 commits into from Apr 10, 2019

Conversation

Projects
None yet
3 participants
@matthiasdusch
Copy link
Member

commented Apr 4, 2019

  • Tests added/passed
  • Fully documented
  • Entry in whats-new.rst

This shall become a download function to use JAXA's ALOS World 3D 30m DEM within OGGM.
I'll put this up here as work in progress to get some comments on how to handle the files being on a ftp server.

@TimoRoth the OGGM file_downloader can not download files from an ftp server out of the box. Do you have any suggestions or ideas how to handle this?

@matthiasdusch

This comment has been minimized.

Copy link
Member Author

commented Apr 4, 2019

Thanks for the comments and for #735 !
I'll finish this tomorrow and yes I agree, we (e.g. I) should tackle the licenses.

@matthiasdusch

This comment has been minimized.

Copy link
Member Author

commented Apr 8, 2019

  • fixed a typo in _classic_urlretrieve
  • finished the AW3D30 downloader

@OGGM/oggm-devs how is the policy on download tests? Should I add one which compares the DEMs?

aw3d30

@fmaussion
Copy link
Member

left a comment

This is nice! Only cosmetic comments from my side.

Regarding tests: we have two downloads tests:

It would be nice to have both tests!

Show resolved Hide resolved docs/whats-new.rst
Show resolved Hide resolved oggm/utils/_downloads.py Outdated
Show resolved Hide resolved oggm/utils/_downloads.py Outdated
Show resolved Hide resolved oggm/utils/_downloads.py Outdated

matthiasdusch added some commits Apr 9, 2019

@matthiasdusch

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2019

Thanks for the review. And I'll add the tests.

Show resolved Hide resolved oggm/utils/_downloads.py Outdated

matthiasdusch and others added some commits Apr 9, 2019

Merge remote-tracking branch 'upstream/master' into jaxa
get relevant commits into this branch
@fmaussion
Copy link
Member

left a comment

LGTM, thanks a lot!

@matthiasdusch

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2019

I added one more test which compares the difference between SRTM and AW3D30. But this might be a bit overkill.

My thinking was, that this would flag with existing but corrupt files or something like that...
Edit: Not corrupt in a typical sens, more like changes to the existing DEM. But I guess that is quite unlikely.

@matthiasdusch

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

decided to remove the SRTM vs. AW3D30 test again

@matthiasdusch matthiasdusch merged commit 8b213af into OGGM:master Apr 10, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 87.445%
Details

@matthiasdusch matthiasdusch deleted the matthiasdusch:jaxa branch Apr 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.