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

Rewrite the meca function to support offsetting and labeling beachballs #1784

Merged
merged 25 commits into from
Jul 3, 2022

Conversation

seisman
Copy link
Member

@seisman seisman commented Mar 4, 2022

Description of proposed changes

In this PR, the core part of the Figure.meca() method is fully rewritten, because the original Figure.meca() implementation is too messy to read. The new implementation takes some inspiration from the original implementation and the changes in PR #1613, especially this comment about passing pandas.DataFrame directly #1613 (comment).

Main changes in the new implementation:

BREAKING CHANGES

  1. Current fix for meca: inaccurate and confusing parameter names for "PRINCIPAL_AXIS" convention #1612 introduces a breaking change, but it's possible to make it backward-compatible. If backward-compatible is required, I can do it in a separate PR, to make this PR small.
  2. When parameters like longitude are passed as the function parameter and are also given in dict/pd.DataFrame (e.g., spec["longitude"]), the behavior changes. In the old implementation, values in spec has high priority; but in the new implementation, values explicitly given as the function parameter override the values in spec. I think the new implementation is more intuitive. The changes are backward-incompatible.

TODO in future PRs:

There are some other meca related issues. I think it would be better to address them in separate PRs because the PR is already too large:

@seisman seisman added this to the 0.6.0 milestone Mar 4, 2022
@seisman seisman added the enhancement Improving an existing feature label Mar 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 10, 2022

Summary of changed images

This is an auto-generated report of images that have changed on the DVC remote

Status Path
added pygmt/tests/baseline/test_meca_dict_eventname.png
added pygmt/tests/baseline/test_meca_dict_offset.png
added pygmt/tests/baseline/test_meca_dict_offset_eventname.png
modified pygmt/tests/baseline/test_meca_spec_dict_list.png

Image diff(s)

Added images

  • pygmt/tests/baseline/test_meca_dict_eventname.png

  • pygmt/tests/baseline/test_meca_dict_offset.png

  • pygmt/tests/baseline/test_meca_dict_offset_eventname.png

Modified images

Path Old New
test_meca_spec_dict_list.png

Report last updated at commit 3c8a1ce

@seisman seisman changed the title New implemention for the meca function Rewrite the meca function to support offset and beachball labels Mar 10, 2022
@seisman seisman changed the title Rewrite the meca function to support offset and beachball labels Rewrite the meca function to support offsetting and labeling beachballs Mar 10, 2022
@seisman seisman modified the milestones: 0.6.0, 0.7.0 Mar 12, 2022
@seisman
Copy link
Member Author

seisman commented Mar 24, 2022

/format

@seisman
Copy link
Member Author

seisman commented Jun 6, 2022

Can someone give this PR a review?

@weiji14
Copy link
Member

weiji14 commented Jun 6, 2022

@jconvers, you mentioned during the EGU PyGMT meetup that you were up for testing code, would you like to try giving this Pull Request a review?

If you're keen, start by installing PyGMT from this branch using pip install https://github.com/GenericMappingTools/pygmt/archive/refactor-meca.zip. After that, try running the example code from https://www.pygmt.org/v0.6.1/gallery/seismology/meca.html and/or make your own beachball plots. Let us know if you need help with anything!

P.S. API docs are at https://pygmt-git-refactor-meca-gmt.vercel.app/api/generated/pygmt.Figure.meca.html

@jconvers
Copy link

jconvers commented Jun 7, 2022

hi @weiji14
I'm definitely up for it. I'll give it a shot.

@jconvers
Copy link

jconvers commented Jun 7, 2022

So... it gave me an error.

Since I don't know if I am supposed to fix it or that its supposed to work by itself, I'm copying the output:
(for reference, I'm using a conda environment made just for testing, with Python 3.10.4 )

pip install https://github.com/GenericMappingTools/pygmt/archive/refactor-meca.zip
Collecting https://github.com/GenericMappingTools/pygmt/archive/refactor-meca.zip
Using cached https://github.com/GenericMappingTools/pygmt/archive/refactor-meca.zip
Installing build dependencies ... done
Getting requirements to build wheel ... done
Installing backend dependencies ... done
Preparing metadata (pyproject.toml) ... done
Collecting netCDF4
Using cached netCDF4-1.5.8-cp310-cp310-macosx_11_0_arm64.whl (464 kB)
Collecting xarray
Using cached xarray-2022.3.0-py3-none-any.whl (870 kB)
Collecting packaging
Using cached packaging-21.3-py3-none-any.whl (40 kB)
Collecting numpy>=1.19
Using cached numpy-1.22.4-cp310-cp310-macosx_11_0_arm64.whl (12.8 MB)
Collecting pandas
Using cached pandas-1.4.2-cp310-cp310-macosx_11_0_arm64.whl (10.1 MB)
Collecting cftime
Using cached cftime-1.6.0-cp310-cp310-macosx_11_0_arm64.whl (195 kB)
Collecting pyparsing!=3.0.5,>=2.0.2
Using cached pyparsing-3.0.9-py3-none-any.whl (98 kB)
Collecting python-dateutil>=2.8.1
Using cached python_dateutil-2.8.2-py2.py3-none-any.whl (247 kB)
Collecting pytz>=2020.1
Using cached pytz-2022.1-py2.py3-none-any.whl (503 kB)
Collecting six>=1.5
Using cached six-1.16.0-py2.py3-none-any.whl (11 kB)
Building wheels for collected packages: pygmt

Building wheel for pygmt (pyproject.toml) ... done
Created wheel for pygmt: filename=pygmt-unknown-py3-none-any.whl size=346940 sha256=96f95ac7f40f3b88c509f6de19618c99a773a43fb35640408cc743fc4e0410fc
Stored in directory: /Users/jconvers/Library/Caches/pip/wheels/3e/b3/d7/b1134329f6075aed42736130e06021a2b5010e6040e7869d92
WARNING: Built wheel for pygmt is invalid: Metadata 1.2 mandates PEP 440 version, but 'unknown' is not
Failed to build pygmt
ERROR: Could not build wheels for pygmt, which is required to install pyproject.toml-based projects

@weiji14
Copy link
Member

weiji14 commented Jun 7, 2022

Oops sorry, wrong command! Try using pip install git+https://github.com/GenericMappingTools/pygmt.git@refactor-meca

@seisman seisman modified the milestones: 0.7.0, 0.8.0 Jun 23, 2022
@weiji14
Copy link
Member

weiji14 commented Jul 2, 2022

@sean0921, I see that you've made a nice GUI for plotting beachballs with meca at https://github.com/sean0921/simplemeca-flask. Do you have time to take a look and review this Pull Request to make sure things work as expected? E.g. using pip install git+https://github.com/GenericMappingTools/pygmt.git@refactor-meca to install PyGMT from this branch. Or if you have any suggestions on what could be improved, that would be useful for us to know as well!

Also re-pinging @jconvers to see if you want to give this another shot. I think the install issues should be resolved now (but let us know if you still encounter errors).

Copy link
Contributor

@sean0921 sean0921 left a comment

Choose a reason for hiding this comment

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

I've roughly check this change with running my project depending on it. It doesn't break my project.
https://imgur.com/a/dVOL20H

If there's no more detail found that could be corrected by me, I will approve these changes.

@weiji14 weiji14 added the final review call This PR requires final review and approval from a second reviewer label Jul 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pygmt.Figure.meca offset not working
4 participants