Skip to content

Add WDMAM dataset to load_earth_magnetic_anomaly #2241

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

Merged
merged 25 commits into from
Dec 29, 2022

Conversation

willschlitzer
Copy link
Contributor

@willschlitzer willschlitzer commented Dec 9, 2022

This adds the wdmam as a data_source to load_earth_magnetic_anomaly to load the World Digital Magnetic Anomaly Map dataset.

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@willschlitzer willschlitzer added the feature Brand new feature label Dec 9, 2022
@willschlitzer willschlitzer added this to the 0.8.0 milestone Dec 9, 2022
@willschlitzer willschlitzer self-assigned this Dec 9, 2022
@willschlitzer willschlitzer changed the title WIP: Add load_earth_wdmam function for WDMAM dataset Add load_earth_wdmam function for WDMAM dataset Dec 9, 2022
@willschlitzer willschlitzer marked this pull request as ready for review December 9, 2022 21:53
@@ -225,6 +225,7 @@ and store them in GMT's user data directory.
datasets.load_earth_geoid
datasets.load_earth_magnetic_anomaly
datasets.load_earth_relief
datasets.load_earth_wdmam
Copy link
Member

Choose a reason for hiding this comment

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

Similar to the comment https://github.com/GenericMappingTools/pygmt/pull/2240/files#r1047080605, the name load_earth_wdmam is not clear, but load_earth_world_digital_magnetic_anomaly_map is too long

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figure that if it's the name for the GMT dataset, it shouldn't be too difficult to use that for the PyGMT function to download it. I think I would feel more concerned if it was commonly used data set like load_earth_relief, but I believe that users looking for the WDMAM dataset will understand the meaning behind load_earth_wdmam.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@GenericMappingTools/pygmt-maintainers Any ideas on a good function name?

Copy link
Member

Choose a reason for hiding this comment

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

Hm, currently not really. The name load_earth_world_digital_magnetic_anomaly_map is quite long. However, the shortcut wdmam is not or only for "experienced" users understandable. load_earth_magnetic_anomaly is used for EMAG2 Global Earth Magnetic Anomaly Model, in which model was skipped. Do we need both earth and world? So maybe load_word_digital_magnetic_anomaly? This is not really short, but at least a bit more in the length range of load_earth_vertical_gravity_gradient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think load_world_digital_magnetic_anomaly could be easily confused with load_earth_magnetic_anomaly.

Copy link
Member

Choose a reason for hiding this comment

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

I know very little about these two datasets, but from the papers (https://doi.org/10.1186/s40623-016-0404-6, https://doi.org/10.1029/2009GC002471):

Note that EMAG2 represents magnetic anomalies at 4 km altitude above the geoid in 2–arc min resolution, while the WDMAM grid was specified at 5 km altitude in 3–arc min resolution.

Conversely, the Earth Magnetic Anomaly Map EMAG-2 (Maus et al. 2009) interpolates between sparse marine magnetic data using directional gridding and extrapolation based on the age map of the seafloor (Müller et al. 2008), but the result displays artificially elongated features due to the improper interpolation of anomalies unrelated to seafloor spreading (associated, for instance, to seamounts).

As I understand them, both EMAG-2 and WDMAM are magnetic anomaly models, but the details diff. So, just like load_earth_relief, perhaps we should add the data_source parameter (valid values are emag2 and wdmam) to the load_earth_magnetic_anomaly function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea.

@yvonnefroehlich yvonnefroehlich mentioned this pull request Dec 20, 2022
65 tasks
@willschlitzer
Copy link
Contributor Author

@GenericMappingTools/pygmt-maintainers I'm going to be away until the 26th and only on phone/website, so please feel free to push any necessary changes to this PR.

@willschlitzer willschlitzer changed the title Add load_earth_wdmam function for WDMAM dataset Add WDMAM dataset to load_earth_magnetic_anomaly Dec 27, 2022
@willschlitzer
Copy link
Contributor Author

I added wdmam= to load_earth_magnetic_anomaly and moved the WDMAM tests to test_datasets_earth_magnetic_anomaly.py.

@willschlitzer willschlitzer added the needs review This PR has higher priority and needs review. label Dec 27, 2022
from pygmt.helpers import kwargs_to_strings

__doctest_skip__ = ["load_earth_magnetic_anomaly"]


@kwargs_to_strings(region="sequence")
def load_earth_magnetic_anomaly(
resolution="01d", region=None, registration=None, mag4km=False
resolution="01d", region=None, registration=None, mag4km=False, wdmam=False
Copy link
Member

@weiji14 weiji14 Dec 28, 2022

Choose a reason for hiding this comment

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

For consistency with load_earth_relief, would it be better to use a single data_source parameter with 3 options (emag, emag4km, wdmam)? Names to be decided.

Comment on lines 64 to 73
- **emag** : EMAG2 Global Earth Magnetic Anomaly Model [Default
option]. Only includes data is observed from sea level over
oceanic regions. See :gmt-datasets:`earth-mag.html`.

- **emag4km** : Use a version of EMAG2 where all observations
are relative to an altitude of 4 km above the geoid and include
data over land

- **wdmam** : World Digital Magnetic Anomaly Map (WDMAM).
See :gmt-datasets:`earth-wdmam.html`
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to the name emag2 instead, because emag2 is the official name of the dataset (https://doi.org/10.7289/V5H70CVX), and emag4km can be renamed to emag2_4km instead.

Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
@weiji14
Copy link
Member

weiji14 commented Dec 28, 2022

/format

Copy link
Member

@weiji14 weiji14 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, thanks @willschlitzer!

@weiji14 weiji14 added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Dec 28, 2022
@weiji14 weiji14 merged commit 5748429 into main Dec 29, 2022
@weiji14 weiji14 deleted the load-remote-dataset/wdmam branch December 29, 2022 03:02
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Brand new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants