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

Add common aliases xshift (X) and yshift (Y) #624

Merged
merged 5 commits into from
Sep 21, 2020
Merged

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Sep 20, 2020

Description of proposed changes

Used for shifting plots along the x and y dimensions. See https://docs.generic-mapping-tools.org/6.1/gmt.html#xy-full.

xshift yshift minimal docs with link to gmt manpage

Live documentation preview for this PR/branch is at https://pygmt-git-common-aliases-xy.gmt.vercel.app/api/generated/pygmt.Figure.plot.html

Note that the xshift/yshift names follow upstream GMT long option name conventions at https://github.com/GenericMappingTools/gmt/blob/88421f9f8fe9759692302905268f4036b287adcb/src/gmt_init.c#L393-L394

Details to address:

  1. Should we have separate {X} and {Y} entries, or a single {XY} entry?
  2. Do we want a) full documentation from https://docs.generic-mapping-tools.org/6.1/gmt.html#xy-full, or b) just the simplified shortened docsting found at https://docs.generic-mapping-tools.org/6.1/std-opts.html?

See related issue at #380. This PR will be a part of #620 too.

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 adding new functionality, add an example to docstrings or tutorials.

@weiji14 weiji14 added the documentation Improvements or additions to documentation label Sep 20, 2020
@seisman
Copy link
Member

seisman commented Sep 20, 2020

  1. Should we have separate {X} and {Y} entries, or a single {XY} entry?

A single {XY} entry looks better.

  1. Do we want a) full documentation from https://docs.generic-mapping-tools.org/6.1/gmt.html#xy-full, or b) just the simplified shortened docsting found at https://docs.generic-mapping-tools.org/6.1/std-opts.html?

All common aliases (e.g., region, projection) are using a simplified docstring. We should follow the convention for X and Y, but we may need to find a better way linking to the full documentation.

@weiji14 weiji14 mentioned this pull request Sep 21, 2020
24 tasks
@weiji14 weiji14 marked this pull request as ready for review September 21, 2020 02:26
pygmt/base_plotting.py Show resolved Hide resolved
Comment on lines +56 to +64
"XY": """\
xshift : str
``[a|c|f|r][xshift]``.
Shift plot origin in x-direction.
yshift : str
``[a|c|f|r][yshift]``.
Shift plot origin in y-direction. Full documentation is at
:gmt-docs:`gmt.html#xy-full`.
""",
Copy link
Member Author

Choose a reason for hiding this comment

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

  1. Do we want a) full documentation from https://docs.generic-mapping-tools.org/6.1/gmt.html#xy-full, or b) just the simplified shortened docsting found at https://docs.generic-mapping-tools.org/6.1/std-opts.html?

All common aliases (e.g., region, projection) are using a simplified docstring. We should follow the convention for X and Y, but we may need to find a better way linking to the full documentation.

Ok, sticking with the simplified docstring, and added a link to the full documentation. Below is how it looks like rendered, let me know if we should expand this a little bit more:

xshift yshift minimal docs with link to gmt manpage

@weiji14 weiji14 changed the title Add common alias xshift (X) and yshift (Y) Add common aliases xshift (X) and yshift (Y) Sep 21, 2020
@seisman seisman added this to the 0.2.1 milestone Sep 21, 2020
@seisman seisman added this to In progress in Release v0.2.x via automation Sep 21, 2020
@weiji14 weiji14 merged commit faf197e into master Sep 21, 2020
Release v0.2.x automation moved this from In progress to Done Sep 21, 2020
@weiji14 weiji14 deleted the common-aliases/XY branch September 21, 2020 03:52
This was referenced Oct 22, 2020
@weiji14 weiji14 mentioned this pull request Nov 6, 2020
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
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants