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

Use "# %%" as code block separators in examples #2662

Merged
merged 41 commits into from
Sep 10, 2023

Conversation

yvonnefroehlich
Copy link
Member

@yvonnefroehlich yvonnefroehlich commented Sep 4, 2023

Description of proposed changes

This PR aims to replace the code block separator ############################################################################### by # %% and also add this separator at thte beginning of the code:

  • Gallery examples
  • Tutorials
  • Intros
  • Projections

Fixes #2660

Preview: https://pygmt-dev--2662.org.readthedocs.build/en/2662/tutorials/basics/coastlines.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
Copy link
Member Author

/format

@yvonnefroehlich yvonnefroehlich self-assigned this Sep 4, 2023
@yvonnefroehlich yvonnefroehlich added the maintenance Boring but important stuff for the core devs label Sep 4, 2023
@seisman
Copy link
Member

seisman commented Sep 7, 2023

Something doesn't work as expected (e.g., https://pygmt-dev--2662.org.readthedocs.build/en/2662/tutorials/basics/coastlines.html):
image

@yvonnefroehlich
Copy link
Member Author

yvonnefroehlich commented Sep 7, 2023

Something doesn't work as expected (e.g., https://pygmt-dev--2662.org.readthedocs.build/en/2662/tutorials/basics/coastlines.html): image

This issue occurs in examples in which we use # sphinx_gallery_thumbnail_number = x.
In commit d375ec5 I changed the order from

"""

# sphinx_gallery_thumbnail_number = x

# %%
import pygmt

to

"""

# %%

# sphinx_gallery_thumbnail_number = x
import pygmt

Then the output looks correct:
update_code_block_separator_sphinx_thumbnail

@seisman
Copy link
Member

seisman commented Sep 8, 2023

Still WIP?

@yvonnefroehlich yvonnefroehlich changed the title WIP: Update code block separators to "# %%" Update code block separators to "# %%" Sep 8, 2023
@yvonnefroehlich yvonnefroehlich added the needs review This PR has higher priority and needs review. label Sep 8, 2023
@yvonnefroehlich
Copy link
Member Author

Still WIP?

Should be ready for review 🙂.

@@ -74,7 +73,8 @@
)
fig.show()

Copy link
Member

@seisman seisman Sep 9, 2023

Choose a reason for hiding this comment

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

The number of blank lines above the # %% separator is inconsistent. For example, there is only one blank line near line 56, but two blank lines near line 77. Maybe we should have only one blank line above # %%, although it really doesn't matter much.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is because there are two blank lines in case of a following subsection, otherwise only one blank is used. This is the concept in some of the tutorials. For me, this makes it easier to read the script, because it gives a bit more structure, especially for longer ones. Thus, I tried / decided to use this consistently across all examples. But as you mentioned, this doesn't matter much, I can change it to have only one blank line before subsections.

Copy link
Member

Choose a reason for hiding this comment

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

This is because there are two blank lines in case of a following subsection, otherwise only one blank is used.

OK. That makes sense to me.

@seisman
Copy link
Member

seisman commented Sep 9, 2023

In commit d375ec5 I changed the order from

"""

# sphinx_gallery_thumbnail_number = x

# %%
import pygmt

to

"""

# %%

# sphinx_gallery_thumbnail_number = x
import pygmt

I had a quick look at the Sphinx-Gallery source code. It seems that anything between the docstring and the first # %% separator is thought as "code". In this case, # sphinx_gallery_thumbnail_number = x will be shown in a code block, but it's also removed because we have remove_config_comments to True in conf.py. That's why we see an empty code block.

Your fix in d375ec5 works as expected, but the biggest limitation is that there must be a blank line between # sphinx_gallery_thumbnail_number = x and # %%.

I think it makes more sense to have # sphinx_gallery_thumbnail_number = x at the end of each script, and it seems work well.

@yvonnefroehlich
Copy link
Member Author

yvonnefroehlich commented Sep 9, 2023

I had a quick look at the Sphinx-Gallery source code. It seems that anything between the docstring and the first # %% separator is thought as "code". In this case, # sphinx_gallery_thumbnail_number = x will be shown in a code block, but it's also removed because we have remove_config_comments to True in conf.py. That's why we see an empty code block.

Your fix in d375ec5 works as expected, but the biggest limitation is that there must be a blank line between # sphinx_gallery_thumbnail_number = x and # %%.

Yes, you are completly right! I was / am also not happy with this blank line. But as I tried several sequences at the beginning of the code and only the one in commit d375ec5 worked without producing an empty code block, I accepted the blank line.

I think it makes more sense to have # sphinx_gallery_thumbnail_number = x at the end of each script, and it seems work well.

I am fine with this solution and can change it later the day. If we go with this solution, I should probably also submit a PR to update the Contributors Guide at https://www.pygmt.org/dev/contributing.html#contributing-tutorials.

@yvonnefroehlich
Copy link
Member Author

I think it makes more sense to have # sphinx_gallery_thumbnail_number = x at the end of each script, and it seems work well.

I am fine with this solution and can change it later the day.

This is done in the commits 2a9c466, d3d5b3b, and d24a45c.

If we go with this solution, I should probably also submit a PR to update the Contributors Guide at https://www.pygmt.org/dev/contributing.html#contributing-tutorials.

Please see PR #2675 for this update.

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.

Look good to me.

@seisman seisman changed the title Update code block separators to "# %%" Update code block separators to "# %%" in gallery examples Sep 9, 2023
@seisman seisman added final review call This PR requires final review and approval from a second reviewer and removed skip-changelog Skip adding Pull Request to changelog needs review This PR has higher priority and needs review. labels Sep 10, 2023
@seisman seisman changed the title Update code block separators to "# %%" in gallery examples Update code block separators to "# %%" in examples Sep 10, 2023
@seisman seisman merged commit 5546573 into main Sep 10, 2023
8 checks passed
@seisman seisman deleted the update-codeblock-separators branch September 10, 2023 10:11
@seisman seisman changed the title Update code block separators to "# %%" in examples Use "# %%" as code block separators in examples Sep 10, 2023
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use "# %%" as code block separators in gallery examples
3 participants