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 gallery example showing different polar projection use cases #955

Merged
merged 53 commits into from
Mar 4, 2021

Conversation

michaelgrund
Copy link
Member

@michaelgrund michaelgrund commented Feb 22, 2021

As mentioned in #942 here's a gallery example showing different ways how the polar projection (-JP) can be used. The example is heavily based on the GMT Chinese documentation.

I would prefer a short discussion about how the parameter descriptions for projection and region should be inlcuded in the figure @GenericMappingTools/python-contributors @GenericMappingTools/python-maintainers. So far I added simple text and adjusted manually since the title option does not support titles with line breaks (as far as I know). Documentation is also missing so far but I will add it after the aforementioned discussion.

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Write detailed docstrings for all functions/methods.

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

Sorry, something went wrong.

@michaelgrund michaelgrund added the documentation Improvements or additions to documentation label Feb 22, 2021
@michaelgrund michaelgrund marked this pull request as draft February 22, 2021 18:11
@seisman seisman added this to the 0.3.1 milestone Feb 22, 2021
@seisman
Copy link
Member

seisman commented Feb 22, 2021

So far I added simple text and adjusted manually since the title option does not support titles with line breaks (as far as I know).

FYI, GMT will support multi-line title and also subtitle in the upcoming 6.2.0 version (see GenericMappingTools/gmt#4562 for how it works).

Currently, I don't know other better ways to add multi-line texts.

@seisman
Copy link
Member

seisman commented Feb 22, 2021

Perhaps easier to put texts in the middle of the figure if you use the justify parameter of Figure.text() (see https://www.pygmt.org/latest/tutorials/text.html#justify-argument).

@michaelgrund
Copy link
Member Author

Perhaps easier to put texts in the middle of the figure if you use the justify parameter of Figure.text() (see https://www.pygmt.org/latest/tutorials/text.html#justify-argument).

Thanks for this, however, it does not work for all examples equally well.

@seisman
Copy link
Member

seisman commented Feb 23, 2021

Perhaps easier to put texts in the middle of the figure if you use the justify parameter of Figure.text() (see pygmt.org/latest/tutorials/text.html#justify-argument).

Thanks for this, however, it does not work for all examples equally well.

Sorry, my previous comment is misleading. I meant the position parameter. For example:

fig.basemap(region=[0, 360, 0, 1], projection="P5c", frame=["xa45f", "+gbisque"])

fig.text(position="TC", text='projection="P5c"', offset="0/2.0c", no_clip=True)
fig.text(position="TC", text="region=[0, 360, 0, 1]", offset="0/1.5c", no_clip=True)

@michaelgrund
Copy link
Member Author

Perhaps easier to put texts in the middle of the figure if you use the justify parameter of Figure.text() (see pygmt.org/latest/tutorials/text.html#justify-argument).

Thanks for this, however, it does not work for all examples equally well.

Sorry, my previous comment is misleading. I meant the position parameter. For example:

fig.basemap(region=[0, 360, 0, 1], projection="P5c", frame=["xa45f", "+gbisque"])

fig.text(position="TC", text='projection="P5c"', offset="0/2.0c", no_clip=True)
fig.text(position="TC", text="region=[0, 360, 0, 1]", offset="0/1.5c", no_clip=True)

Works better but there are still some issues:

  1. When using text='projection="P5c"' the " " are not displayed in the figure. Instead I used "' '" which then results in projection='P5c' in the plot.
  2. If only a single option is appended (e.g. text="projection='P5c+a'") everything is displayed. However, when appending further options (e.g. text="projection='P5c+a+t45'") nothing is displayed in the figure. Maybe an upstream issue?

@seisman
Copy link
Member

seisman commented Feb 24, 2021

Works better but there are still some issues:

  1. When using text='projection="P5c"' the " " are not displayed in the figure. Instead I used "' '" which then results in projection='P5c' in the plot.

I think it's caused by how we pass arguments to GMT API, similar to #247.

  1. If only a single option is appended (e.g. text="projection='P5c+a'") everything is displayed. However, when appending further options (e.g. text="projection='P5c+a+t45'") nothing is displayed in the figure. Maybe an upstream issue?

Related to GenericMappingTools/gmt#576. The main reason is that GMT may think "+t" is an option modifier, not part of a long string.

@seisman
Copy link
Member

seisman commented Feb 25, 2021

  1. If only a single option is appended (e.g. text="projection='P5c+a'") everything is displayed. However, when appending further options (e.g. text="projection='P5c+a+t45'") nothing is displayed in the figure. Maybe an upstream issue?

Related to GenericMappingTools/gmt#576. The main reason is that GMT may think "+t" is an option modifier, not part of a long string.

If you change +45 to \+t45, it will work.

@michaelgrund
Copy link
Member Author

  1. If only a single option is appended (e.g. text="projection='P5c+a'") everything is displayed. However, when appending further options (e.g. text="projection='P5c+a+t45'") nothing is displayed in the figure. Maybe an upstream issue?

Related to GenericMappingTools/gmt#576. The main reason is that GMT may think "+t" is an option modifier, not part of a long string.

If you change +45 to \+t45, it will work.

Thanks @seisman !

examples/projections/nongeo/polar-misc.py Outdated Show resolved Hide resolved
examples/projections/nongeo/polar-misc.py Outdated Show resolved Hide resolved
examples/projections/nongeo/polar-misc.py Outdated Show resolved Hide resolved
Copy link
Member

@seisman seisman left a comment

Choose a reason for hiding this comment

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

@GenericMappingTools/python @GenericMappingTools/python-maintainers

This gallery example looks good to me. Please give it a second-round review.

examples/projections/nongeo/polar-misc.py Outdated Show resolved Hide resolved
Copy link
Member

@maxrjones maxrjones left a comment

Choose a reason for hiding this comment

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

Nice! Given how detailed this gallery example is and that it contains all the information presented in the gallery example titled 'Polar', I think it would be more clear if it were the only polar non-geographic projection example and the other example were removed. The title for this example could then simply be 'Polar' for consistency with the other projection examples.

examples/projections/nongeo/polar-misc.py Outdated Show resolved Hide resolved
@michaelgrund
Copy link
Member Author

Nice! Given how detailed this gallery example is and that it contains all the information presented in the gallery example titled 'Polar', I think it would be more clear if it were the only polar non-geographic projection example and the other example were removed. The title for this example could then simply be 'Polar' for consistency with the other projection examples.

Would be fine for me. Any other comments on this?

@seisman
Copy link
Member

seisman commented Mar 3, 2021

Nice! Given how detailed this gallery example is and that it contains all the information presented in the gallery example titled 'Polar', I think it would be more clear if it were the only polar non-geographic projection example and the other example were removed. The title for this example could then simply be 'Polar' for consistency with the other projection examples.

Would be fine for me. Any other comments on this?

Sounds good.

@michaelgrund michaelgrund linked an issue Mar 4, 2021 that may be closed by this pull request
@michaelgrund
Copy link
Member Author

Nice! Given how detailed this gallery example is and that it contains all the information presented in the gallery example titled 'Polar', I think it would be more clear if it were the only polar non-geographic projection example and the other example were removed. The title for this example could then simply be 'Polar' for consistency with the other projection examples.

Would be fine for me. Any other comments on this?

Sounds good.

Committed the proposed changes.

@seisman seisman marked this pull request as ready for review March 4, 2021 15:21
@michaelgrund
Copy link
Member Author

Thanks for the careful and detailed review @seisman and @meghanrjones .

@seisman seisman merged commit f57943f into master Mar 4, 2021
@seisman seisman deleted the misc-polar-projs branch March 4, 2021 18:14
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
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.

Add more examples for polar projections
3 participants