-
Notifications
You must be signed in to change notification settings - Fork 224
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
Allow load_earth_relief() to load the original land-only 01s or 03s SRTM tiles #976
Allow load_earth_relief() to load the original land-only 01s or 03s SRTM tiles #976
Conversation
@core-man I just converted your PR to draft to save CI resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to add a test to make sure that the change work as expected.
Please see pygmt/tests/test_datasets.py
for similar tests.
|
@core-man Sorry, I didn't realize that you already added a test to We just split the tests in Sorry for the trouble. Please let me know if you need any help resolving the conflicts. |
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
ec100c8
to
a3a6a05
Compare
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Ping @GenericMappingTools/python for further comments.
FYI, we're planning to make a patch release (v0.3.1) in mid-March. Since this PR adds a new feature, it should not be merged into the v0.3.1 release. We will merge this PR after the v0.3.1 release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be labelled as a 'feature' or 'enhancement'? I feel like we can sneak it in for v0.3.1, but also ok with saving if for v0.4.0 a month later.
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
This PR is ready to merge. @weiji14 @michaelgrund @willschlitzer @meghanrjones Please see if you have further comments. |
…RTM tiles (GenericMappingTools#976) Allow the load_earth_relief function to load the original land-only 01s or 03s SRTM tiles, by adding a new parameter `use_srtm` (default to False). Co-authored-by: Dongdong Tian <seisman.info@gmail.com> Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
Description of proposed changes
Allow the load_earth_relief function to load the original land-only SRTM tiles, which have a resolution of
01s
or03s
.A new parameter
use_srtm
(default toFalse
) was added to perform this feature.If
resolution
is01s
or03s
, we then checkuse_srtm
to determine the relief data prefix (earth_relief_prefix
).use_srtm
isFalse
,earth_relief_prefix
=earth_relief_
.use_srtm
isTrue
,earth_relief_prefix
=srtm_relief_
.Fixes #966
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version