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

Wrap grdinfo aliases #799

Merged
merged 49 commits into from
Feb 7, 2021

Conversation

willschlitzer
Copy link
Contributor

@willschlitzer willschlitzer commented Jan 21, 2021

Wrapping the R, C, D, F, I, L, T, and M aliases for grdinfo. This is the first part of improving grdinfo, as mentioned in #593 , and once this is done I intend to create an argument to return a dictionary of the grdinfo values.

Ref GMT.jl at https://www.generic-mapping-tools.org/GMT.jl/v0.28/#GMT.grdinfo

Documentation preview is at https://pygmt-git-fork-willschlitzer-grdinfo-alias.gmt.vercel.app/api/generated/pygmt.grdinfo.html#pygmt.grdinfo

pygmt/modules.py Outdated Show resolved Hide resolved
@weiji14 weiji14 added the documentation Improvements or additions to documentation label Jan 21, 2021
@willschlitzer
Copy link
Contributor Author

@seisman I think I have taken care of all of your recommended changes

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.

Just some more minor suggestions :)

pygmt/modules.py Outdated Show resolved Hide resolved
pygmt/src/grdinfo.py Outdated Show resolved Hide resolved
pygmt/src/grdinfo.py Outdated Show resolved Hide resolved
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.

One more tick off the list 😄

pygmt/src/grdinfo.py Outdated Show resolved Hide resolved
Co-authored-by: Dongdong Tian <[email protected]>
@weiji14
Copy link
Member

weiji14 commented Feb 6, 2021

@willschlitzer, could you squash and merge this yourself? Remember to summarize the commit message!

@willschlitzer
Copy link
Contributor Author

@GenericMappingTools/python It looks like this keeps failing Windows tests (but not the same tests). Is there a particular method we use to squash and merge, or should I just follow the command line instructions that GitHub recommends?

@weiji14
Copy link
Member

weiji14 commented Feb 7, 2021

@GenericMappingTools/python It looks like this keeps failing Windows tests (but not the same tests).

It's ok to ignore the random Windows test failures (we've been trying unsuccessfully to debug and fix it for a while now, see #758).

Is there a particular method we use to squash and merge, or should I just follow the command line instructions that GitHub recommends?

Just use the Github UI (i.e. click the 'squash and merge' button). Do not use the command line instructions as it does a slightly different thing. I'll disable the required Windows checks so you can merge (after resolving the conflict on pygmt/src/__init__.py).

@weiji14
Copy link
Member

weiji14 commented Feb 7, 2021

/format

@willschlitzer willschlitzer merged commit 073a83a into GenericMappingTools:master Feb 7, 2021
@willschlitzer willschlitzer deleted the grdinfo-alias branch February 7, 2021 14:12
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
*Wrap grdinfo R, C, D, F, I, L, T, and M aliases
*Move grdinfo function to grdinfo.py

Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: actions-bot <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants