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

Tutorial "Plotting text": Rewrite to improve structure, explain more parameters, show list input #2760

Merged
merged 57 commits into from
Apr 21, 2024

Conversation

yvonnefroehlich
Copy link
Member

@yvonnefroehlich yvonnefroehlich commented Oct 20, 2023

Description of proposed changes

This PR aims to expand and update the tutorial "Plotting text":

  • Mention the position parameter along with justify
  • Mention the pen parameter along with fill
  • Grouping and showing the configurations justify, font, angle, fill + pen via simple Cartesian plots
  • Showing the possiblity of providing lists for justify, font, and angle (possible after PR Figure.text: Support passing in a list of angle/font/justify values #2720) beside providing an external input file

Please state, whether these changes are improvements, or that we should keep the current version.

Changes suggested by @seisman to improve the structure of the tutorial (see comment #2760 (comment)):

  • 1. The first map should have only one text (e.g., text="SOUTH CHINA SEA", x=112, y=6)
  • 2. Introduce the angle/font/justify/pen/fill parameters to adjust the texts (maybe one map for angle/font/justify and another map for pen/fill
  • 3. Show how to add several texts in one Figure.text call (e.g., pass 1-D arrays to x/y/text)
  • 4. Show that angle/font/justify can also accept 1-D arrays (maybe combine 3 and 4 into one map)
  • 5. Explain that it's also possible to pass a file or a 2-D matrix to data textfiles
  • 6. Introduce the position parameter. A useful use case for this parameter is to add a tag (e.g., (a)) at the top left corner of the map for multiple subplots, but we should also mention that the Figure.subplot method can do this automatically.

Preview: https://pygmt-dev--2760.org.readthedocs.build/en/2760/tutorials/basics/text.html

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 wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.
  • Use underscores (not hyphens) in names of Python files and directories.

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

@yvonnefroehlich yvonnefroehlich self-assigned this Oct 20, 2023
@yvonnefroehlich yvonnefroehlich added the documentation Improvements or additions to documentation label Oct 20, 2023
@yvonnefroehlich yvonnefroehlich added this to the 0.11.0 milestone Oct 20, 2023
@yvonnefroehlich yvonnefroehlich changed the title WIP: Tutorial "Plotting text": Update and expand regarding parameters and list inputs Tutorial "Plotting text": Update and expand regarding parameters and list inputs Oct 22, 2023
@yvonnefroehlich yvonnefroehlich added the needs review This PR has higher priority and needs review. label Oct 25, 2023
@seisman
Copy link
Member

seisman commented Oct 26, 2023

Sorry to be mean here, but I think the "Plotting text" tutorial (both the old version and the current new version) is not as good as other tutorials. In other tutorials, we usually start with a simple plot with a few required parameters and then improve the plot by adding more parameters step by step in the following sections, but in this tutorial, we start with a nice map and then start to explain the position/text, font, angle, fill/pen parameters, which has nothing to do with the first map. The tutorial reads more like a manual for Figure.text's parameters, rather than a tutorial for adding texts.

Here are my thoughts about the tutorial:

  1. The first map should have only one text (e.g., text="SOUTH CHINA SEA", x=112, y=6)
  2. Introduce the angle/font/justify/pen/fill parameters to adjust the texts (maybe one map for angle/font/justify and another map for pen/fill
  3. Show how to add several texts in one Figure.text call (e.g., pass 1-D arrays to x/y/text)
  4. Show that angle/font/justify can also accept 1-D arrays (maybe combine 3 and 4 into one map)
  5. Explain that it's also possible to pass a file or a 2-D matrix to data
  6. Introduce the position parameter. A useful use case for this parameter is to add a tag (e.g., (a)) at the top left corner of the map for multiple subplots, but we should also mention that the Figure.subplot method can do this automatically.

@yvonnefroehlich
Copy link
Member Author

Sorry to be mean here, but I think the "Plotting text" tutorial (both the old version and the current new version) is not as good as other tutorials. In other tutorials, we usually start with a simple plot with a few required parameters and then improve the plot by adding more parameters step by step in the following sections, but in this tutorial, we start with a nice map and then start to explain the position/text, font, angle, fill/pen parameters, which has nothing to do with the first map. The tutorial reads more like a manual for Figure.text's parameters, rather than a tutorial for adding texts.

Here are my thoughts about the tutorial:

  1. The first map should have only one text (e.g., text="SOUTH CHINA SEA", x=112, y=6)
  2. Introduce the angle/font/justify/pen/fill parameters to adjust the texts (maybe one map for angle/font/justify and another map for pen/fill
  3. Show how to add several texts in one Figure.text call (e.g., pass 1-D arrays to x/y/text)
  4. Show that angle/font/justify can also accept 1-D arrays (maybe combine 3 and 4 into one map)
  5. Explain that it's also possible to pass a file or a 2-D matrix to data
  6. Introduce the position parameter. A useful use case for this parameter is to add a tag (e.g., (a)) at the top left corner of the map for multiple subplots, but we should also mention that the Figure.subplot method can do this automatically.

No worries @seisman. My initial intent for this PR was only to add a code example regarding list input for font, angle, and justify. But this turned out to be more difficult than expected because of the current structure of the tutorial. Thus, I made some additional changes to the tutorial and then tried to include the list-input-aspect. But as you mentioned, the tutorial is still not optimal in terms of its structure from an educational perspective. However, I was unsure about making more significant changes, as I am not the original author of this tutorial. But I am open to consider your suggested points above to restructure and rewrite this tutorial. Maybe it makes sense to see what the other @GenericMappingTools/pygmt-maintainers think about these changes🙂 before I start working on this.

@yvonnefroehlich yvonnefroehlich removed the needs review This PR has higher priority and needs review. label Oct 27, 2023
@yvonnefroehlich yvonnefroehlich marked this pull request as draft October 29, 2023 16:49
@yvonnefroehlich yvonnefroehlich changed the title Tutorial "Plotting text": Update and expand regarding parameters and list inputs WIP: Tutorial "Plotting text": Rewrite to improve structure, add parameter, add list input Oct 29, 2023
@yvonnefroehlich yvonnefroehlich changed the title WIP: Tutorial "Plotting text": Rewrite to improve structure, add parameter, add list input WIP: Tutorial "Plotting text": Rewrite to improve structure, add parameters, add list input Oct 29, 2023
@seisman seisman removed this from the 0.12.0 milestone Feb 26, 2024
@yvonnefroehlich yvonnefroehlich added the needs review This PR has higher priority and needs review. label Apr 13, 2024
@yvonnefroehlich
Copy link
Member Author

yvonnefroehlich commented Apr 14, 2024

I tried to address the issues mentioned by @seisman in #2760 (comment). @GenericMappingTools/pygmt-maintainers do you think the current changes are improving this tutorial?

examples/tutorials/basics/text.py Outdated Show resolved Hide resolved
examples/tutorials/basics/text.py Outdated Show resolved Hide resolved
examples/tutorials/basics/text.py Outdated Show resolved Hide resolved
yvonnefroehlich and others added 4 commits April 19, 2024 10:01
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
@yvonnefroehlich yvonnefroehlich mentioned this pull request Apr 19, 2024
35 tasks
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.

Looks good.

examples/tutorials/basics/text.py Outdated Show resolved Hide resolved
@seisman seisman added this to the 0.12.0 milestone Apr 20, 2024
@seisman seisman added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Apr 20, 2024
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
@seisman seisman merged commit f339678 into main Apr 21, 2024
9 checks passed
@seisman seisman deleted the add-lists-tut-text branch April 21, 2024 05:41
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Apr 21, 2024
@yvonnefroehlich yvonnefroehlich changed the title Tutorial "Plotting text": Rewrite to improve structure, add parameters, add list input Tutorial "Plotting text": Rewrite to improve structure, explain more parameters, show list input Apr 30, 2024
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.

3 participants