Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

DOC: improve spines crosslinking #23316

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

Merged
merged 1 commit into from
Jun 25, 2022

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Jun 21, 2022

PR Summary

It is really hard to find the Spine class from the examples given, and it is hard to find the examples from the Spine class. This attempts to improve that...

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

@jklymak jklymak force-pushed the doc-improve-spines-crosslink branch 2 times, most recently from ba8d224 to e071856 Compare June 21, 2022 08:51
@jklymak jklymak marked this pull request as ready for review June 21, 2022 09:29
@@ -8,6 +8,10 @@
- normal Axes, with spines on all four sides;
- an Axes with spines only on the left and bottom;
- an Axes using custom bounds to limit the extent of the spine.

Each `.axes.Axes` has a list of `~.Spine` objects, accessible
Copy link
Member

Choose a reason for hiding this comment

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

can this be formatted like the references section in https://matplotlib.org/stable/gallery/ticks/tick-formatters.html ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what you are after here?

Copy link
Member

Choose a reason for hiding this comment

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

I think something like this makes it clearer what the api reference is than having it written out in the prose
image
but reading through again, this might be an in addition to rather than instead of suggestion (sorry!)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, that's a good suggestion. Newest version has that at the bottom now.

@@ -8,6 +8,10 @@
- normal Axes, with spines on all four sides;
- an Axes with spines only on the left and bottom;
- an Axes using custom bounds to limit the extent of the spine.

Each `.axes.Axes` has a list of `~.Spine` objects, accessible
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Each `.axes.Axes` has a list of `~.Spine` objects, accessible
Each `.axes.Axes` has a list of `.Spine` objects, accessible

The only purpose of ~ is shortening link text to the last component of the given qualified name. So ~.Something is exactly the same as .Something and the latter is easier to read in the raw docstring.

Copy link
Member Author

Choose a reason for hiding this comment

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

I mildly disagree - ~.Spine makes it clear to me that its relative to "home", eg the top level of the library, whereas .Spine makes me think its a submodule of the current module. I appreciate that's not how sphinx handles it, but when reading the raw docstring that's how I always think about it.

Copy link
Member

@timhoffm timhoffm Jun 25, 2022

Choose a reason for hiding this comment

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

Unfortunately, associating ~ with "home" is a false analogy. Coneceptually, ~ has nothing to do with the "path"/fully qualified name. It only affects the displayed text of the link. I advise against trying to use it for other purposes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Duh, sorry, you are right. I should have my coffee before reading GitHub.

Comment on lines 13 to 14
as ``ax.spines.left``, ``ax.spines.right``, ``ax.spines.bottom``,
and ``ax.spines.top``.
Copy link
Member

Choose a reason for hiding this comment

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

ax.spines.left and similar is only part of the spine accessor functionality. I suggest to link https://matplotlib.org/stable/api/spines_api.html#matplotlib.spines.Spines instead:

Suggested change
as ``ax.spines.left``, ``ax.spines.right``, ``ax.spines.bottom``,
and ``ax.spines.top``.
via the container `ax.spines`. See `.Spines`.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't do the See .Spines as we had just linked Spine in the sentence....

@jklymak jklymak force-pushed the doc-improve-spines-crosslink branch from 5821744 to daa24b0 Compare June 25, 2022 14:31
@timhoffm timhoffm added this to the v3.6.0 milestone Jun 25, 2022
@timhoffm timhoffm merged commit 0d39a4f into matplotlib:main Jun 25, 2022
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.

3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.