-
Notifications
You must be signed in to change notification settings - Fork 234
Rewrite the meca function to support offsetting and labeling beachballs #1784
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
Conversation
Summary of changed imagesThis is an auto-generated report of images that have changed on the DVC remote
Image diff(s)Added images
Modified images
Report last updated at commit 3c8a1ce |
|
/format |
|
Can someone give this PR a review? |
|
@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 P.S. API docs are at https://pygmt-git-refactor-meca-gmt.vercel.app/api/generated/pygmt.Figure.meca.html |
|
hi @weiji14 |
|
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: pip install https://github.com/GenericMappingTools/pygmt/archive/refactor-meca.zip Building wheel for pygmt (pyproject.toml) ... done |
|
Oops sorry, wrong command! Try using |
|
@sean0921, I see that you've made a nice GUI for plotting beachballs with 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). |
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.
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.
…ls (GenericMappingTools#1784) Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
Description of proposed changes
In this PR, the core part of the
Figure.meca()method is fully rewritten, because the originalFigure.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 passingpandas.DataFramedirectly #1613 (comment).Main changes in the new implementation:
virtualfile_from_datafunction to pass data (address Universal virtualfile_from_data function to replace virtualfile_from_grid/matrix/vectors #949)pandas.DataFrameto thevirtualfile_from_datadirectly, becausepandas.DataFrameallows columns with different data types (proposed in Refactor meca to use virtualfile_from_data #1613 (comment) and works well)plot_longitudeandplot_latitudecolumns/parameters (fixes pygmt.Figure.meca offset not working #1129)event_namecolumn/parameter (address Wrap meca #516 (comment)). [Here I useevent_namerather thanlabel, becauselabelis usually aliased to GMT's-loption, and there is a plan to add-ltomecaxref https://github.com/Modules that should take -l for auto-legend building gmt#1973)BREAKING CHANGES
longitudeare 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 inspechas high priority; but in the new implementation, values explicitly given as the function parameter override the values inspec. I think the new implementation is more intuitive. The changes are backward-incompatible.TODO in future PRs:
There are some other
mecarelated issues. I think it would be better to address them in separate PRs because the PR is already too large:longitudeandlatitudehave the same length)