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

Rewrite Tick formatters example #26068

Merged
merged 1 commit into from Jun 29, 2023

Conversation

timhoffm
Copy link
Member

@timhoffm timhoffm commented Jun 4, 2023

  • Collect all relevant descriptions in the summary text at the top
  • Merge everything into one figure. The code should be mostly irrelevant for the example. The figure can serve as a standalone reference.

Also closes 25967 by deleting the "Dollar ticks" example. [Deferred]

@timhoffm timhoffm added this to the v3.8.0 milestone Jun 4, 2023
Copy link
Member

@story645 story645 left a comment

Choose a reason for hiding this comment

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

Since it's all getting combined and the code doesn't matter, is there anyway to loop over the arguments so that the titles have to stay in sync w/ the code?

galleries/examples/ticks/tick-formatters.py Outdated Show resolved Hide resolved
@timhoffm
Copy link
Member Author

timhoffm commented Jun 4, 2023

Since it's all getting combined and the code doesn't matter, is there anyway to loop over the arguments so that the titles have to stay in sync w/ the code?

I don't think so. While https://matplotlib.org/stable/gallery/ticks/date_formatters_locators.html does something like that, the special code for FuncFormatter and FixedFormatter make it difficult here.

I'm not at all worried about "stay in sync". This will hardly change and the title and formatter code are always in neighboring lines.

@story645
Copy link
Member

story645 commented Jun 5, 2023

My own bias, but can you put the figure before the code since the code isn't the important part?

@timhoffm
Copy link
Member Author

timhoffm commented Jun 5, 2023

AFAIK this is not possible in sphinx-gallery. What I would like even more would be an option to collapse code blocks in sphinx-gallery, but that's also not supported.

@rcomer
Copy link
Member

rcomer commented Jun 5, 2023

According to this page, if there is only one code block the image appears before the code.
https://sphinx-gallery.github.io/stable/auto_examples/plot_1_exp.html#sphx-glr-auto-examples-plot-1-exp-py

I am not clear how it is counting more than one code block for the current case 😕

@timhoffm
Copy link
Member Author

timhoffm commented Jun 6, 2023

Likely it counts:

In this example we have the References text block after the code. Likely that prevents the reordering, which makes sense because it would bring two description blocks together that we have written separately.

should I move this up? If so, I would reduce it to the set-*-formatter references. The references section is mainly there to generate the backreferences mini-galleries. This is anyway not happening on the formatter classes and not important for the other functions mentioned.

@story645
Copy link
Member

story645 commented Jun 6, 2023

should I move this up? If so, I would reduce it to the set-*-formatter references.

I think you can probably drop it here since that's more or less what you're frontloading with the text. I think having a more prominent figure should be the priority since that's kinda the important part of this example (as an aside/out of scope here though, maybe the figure should go into the ticker docs since it's a reference for the formatters and same for the locators)

@rcomer
Copy link
Member

rcomer commented Jun 6, 2023

I think the reference section is no longer needed to generate the mini-galleries. For example plt.rcdefaults's gallery shows two examples that do not have the explicit reference list. These days the functions within the code work as clickable links, so maybe it gets it from that?

@timhoffm
Copy link
Member Author

timhoffm commented Jun 6, 2023

It depends. Sphinx-gallery cannot infer all objects. For example, ax0.xaxis.set_major_formatter is linked, but axs2[0].xaxis.set_major_formatter is not. Luckily, we here have links for all relevant functions in the code, so the references section can indeed go.

@timhoffm
Copy link
Member Author

timhoffm commented Jun 6, 2023

The references section is gone.

I've additionally removed the "Dollar ticks" example (closes #25967). The formatting is more concisely explained in this overview. We don't need individual examples for all the formatters.

@rcomer
Copy link
Member

rcomer commented Jun 12, 2023

The dollar ticks example is included here:

# Here is an example which sets the formatter for the right side ticks with
# dollar signs and colors them green on the right side of the yaxis.
#
#
# .. include:: ../gallery/ticks/dollar_ticks.rst
# :start-after: .. redirect-from:: /gallery/pyplots/dollar_ticks
# :end-before: .. admonition:: References

@timhoffm timhoffm marked this pull request as draft June 26, 2023 13:36
@timhoffm
Copy link
Member Author

I've restored the dollar ticks example. Let's handle this in another PR.

@timhoffm timhoffm marked this pull request as ready for review June 29, 2023 07:31
- Collect all relevant descriptions in the summary text at the top
- Merge everything into one figure. The code should be mostly
  irrelevant for the example. The figure can serve as a
  standalone reference.
Comment on lines +26 to +27
See :ref:`formatters` for a complete list.

Copy link
Member

@story645 story645 Jun 29, 2023

Choose a reason for hiding this comment

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

Suggested change
See :ref:`formatters` for a complete list.
See :ref:`formatters` for a complete list.

Take or leave unindenting this so that this is a bit more prominent. Can self merge

Copy link
Member Author

Choose a reason for hiding this comment

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

The list is a list of formatters. So indentation is correct IMHO

@timhoffm timhoffm merged commit 7cab499 into matplotlib:main Jun 29, 2023
37 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants