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

Update the starter tutorial introduction #1607

Merged
merged 55 commits into from
Dec 20, 2021
Merged

Conversation

willschlitzer
Copy link
Contributor

@willschlitzer willschlitzer commented Nov 5, 2021

This pull request adds an .rst file for the introduction to the starter tutorial to allow for additional starting tutorial pages to be added. It is the first in a series of pull requests to add a starter tutorial that expands beyond first_figure.py.

Addresses #770

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.

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

@willschlitzer willschlitzer added the documentation Improvements or additions to documentation label Nov 5, 2021
@willschlitzer willschlitzer added this to the 0.6.0 milestone Nov 5, 2021
@willschlitzer willschlitzer self-assigned this Nov 5, 2021
@willschlitzer
Copy link
Contributor Author

willschlitzer commented Nov 5, 2021

image
@GenericMappingTools/pygmt-admin I tried following what GMT used for setting up it's multi-page tutorial, but the link for the tutorial intro (tutorials/starter-tutorial.rst) is not showing up in the deployed environment. Any idea how to fix this?

@maxrjones
Copy link
Member

image @GenericMappingTools/pygmt-admin I tried following what GMT used for setting up it's multi-page tutorial, but the link for the tutorial intro (tutorials/starter-tutorial.rst) is not showing up in the deployed environment. Any idea how to fix this?

You need to update doc/conf.py for your new folder (e.g., https://github.com/GenericMappingTools/pygmt/pull/1603/files#diff-e170e9a7d787c21095c6c11bb25f0f1ff0294a42a46d45ba6fb5ed794e457624). There will be merge conflicts between this and main if #1603 gets merged, so you may want to wait to resolve those before making more updates.

@willschlitzer
Copy link
Contributor Author

You need to update doc/conf.py for your new folder (e.g., https://github.com/GenericMappingTools/pygmt/pull/1603/files#diff-e170e9a7d787c21095c6c11bb25f0f1ff0294a42a46d45ba6fb5ed794e457624). There will be merge conflicts between this and main if #1603 gets merged, so you may want to wait to resolve those before making more updates.

Thanks @meghanrjones! I wait until #1603 is merged and then I'll get this sorted.

@willschlitzer
Copy link
Contributor Author

willschlitzer commented Nov 15, 2021

image

I manually merged the main branch and update doc/conf.py to include the starter tutorials folder, but I'm still not displaying the link to starter-tutorial.rst. I'm obviously missing some key detail here, but I'm not able to figure out where I went wrong (cue @meghanrjones and @weiji14 pointing out a very easy fix 😆 ).

To be clear, I'm planning on having this follow the model used on the GMT documentation, in that there is a link to the tutorial main page, which has links to individual pages/examples, otherwise the entire tutorial would be a big file and cumbersome page.

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.

Here are some steps that will make this show in the "Getting Started" section and not under the "User Guide/tutorials" section:

  1. Commit suggested changes
  2. From the base of the repository:
    mkdir examples/starter-tutorial
    git mv examples/tutorials/starter-tutorial.rst examples/starter-tutorial/README.txt
    git mv examples/tutorials/starter-tutorials/first_figure.py examples/starter-tutorial/first_figure.py
    rmdir examples/tutorials/starter-tutorials/
    
  3. Add "../examples/starter-tutorial", after L68 in doc/conf.py (in example_dirs)
  4. In L72 in doc/conf.py, change:
    "gallery_dirs": ["gallery", "tutorials", "projections"],
    to
    "gallery_dirs": ["gallery", "tutorials", "starter-tutorial", "projections"],
  5. Add doc/starter-tutorial/ to .gitignore
  6. Add rm -rf starter-tutorial to L51 in doc/Makefile

I made these changes locally to try it out. If you want, I could commit and push them to this branch to save time (and revert the changes later if you do not like the structure).

doc/conf.py Outdated Show resolved Hide resolved
doc/index.rst Outdated Show resolved Hide resolved
examples/tutorials/starter-tutorial.rst Outdated Show resolved Hide resolved
@willschlitzer
Copy link
Contributor Author

It worked! Thanks a ton for the help, @meghanrjones!

examples/get-started/first_figure.py Outdated Show resolved Hide resolved
examples/get-started/first_figure.py Outdated Show resolved Hide resolved
examples/get-started/first_figure.py Outdated Show resolved Hide resolved
examples/get-started/first_figure.py Outdated Show resolved Hide resolved
examples/get-started/first_figure.py Outdated Show resolved Hide resolved
examples/get-started/first_figure.py Outdated Show resolved Hide resolved
examples/get-started/first_figure.py Outdated Show resolved Hide resolved
#
# 4. Create a global map. Set the region to "d" to center the map at the Prime
# Meridian or "g" to center the map at the International Date Line. When the
# region is set without using a list full of integers or floating numbers,
Copy link
Member

Choose a reason for hiding this comment

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

When the region is set without using a list full of integers or floating numbers, the argument needs to be passed as a Python string.

If I understand you correctly, you mean passing a string like -20/20/-30/30 to region, right? I don't think it's clearly explained by this sentence because it doesn't say that the string should contain slash-separated numbers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm actually referring to passing d or g when it comes to region. While I know that the literal GMT string can be passed for region, I don't want to make a mention of it in the first section of the tutorial. I think bringing up too many options early on is confusing, and using a list for something like this is probably more familiar to Python users instead of a string with numbers.

Copy link
Member

Choose a reason for hiding this comment

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

I see what you meant. I also agree that a string like -20/20/-30/30 is not Pythonic and should not be covered in this starter tutoria.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, you could pass in a list of strings like ["10W", "20E", "30S", "40N"] right? Not necessarily all numbers.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, you could pass in a list of strings like ["10W", "20E", "30S", "40N"] right? Not necessarily all numbers.

Yes, but it's not commonly used, and better not to introduce too many options.

@seisman seisman added the final review call This PR requires final review and approval from a second reviewer label Dec 15, 2021
Co-authored-by: TIAN Dongdong <[email protected]>
Co-authored-by: Michael Grund <[email protected]>
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.

Nice work!


**About this tutorial**

This tutorial assumes that PyGMT has been successfully installed. A quick
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a link to the install instructions here. (Is there a way to do relative links?)

Suggested change
This tutorial assumes that PyGMT has been successfully installed. A quick
This tutorial assumes that PyGMT has been successfully [installed](https://www.pygmt.org/latest/install.html). A quick

Copy link
Member

Choose a reason for hiding this comment

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

The following directive should work:

:doc:`install`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do I add the text that I want to be linked ("installed")?

Copy link
Member

Choose a reason for hiding this comment

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

:doc:`installed <install>`

or

:doc:`installed </install>`

should work.

examples/get-started/README.txt Outdated Show resolved Hide resolved
examples/get-started/first_figure.py Outdated Show resolved Hide resolved
examples/get-started/first_figure.py Outdated Show resolved Hide resolved
examples/get-started/first_figure.py Show resolved Hide resolved
examples/get-started/first_figure.py Outdated Show resolved Hide resolved
@@ -0,0 +1,155 @@
"""
1. Making your first figure
Copy link
Member

Choose a reason for hiding this comment

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

Just as a thought, should we add a binder link so that people can 'follow along' with the tutorial interactively?

Copy link
Contributor Author

@willschlitzer willschlitzer Dec 17, 2021

Choose a reason for hiding this comment

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

@weiji14 I'm not familiar with using binder; could you make a code suggestion to add it?

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it's better to add the binder link in a separate PR. Ideally, we should modify the template to automatically add the binder links for each example.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, sounds good to me.

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.

Nice work, thanks @willschlitzer!

@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Dec 20, 2021
@seisman seisman merged commit 59eabe3 into main Dec 20, 2021
@seisman seisman deleted the tutorial/starter-tutorial branch December 20, 2021 07:27
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
Co-authored-by: Meghan Jones <[email protected]>
Co-authored-by: Michael Grund <[email protected]>
Co-authored-by: Dongdong Tian <[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.

5 participants