Skip to content

Demonstrate inset_axes in scatter_hist example. #21283

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
Oct 16, 2021
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Oct 5, 2021

Currently, the scatter_hist example needs to force the main axes to be
square by carefully adjusting the figure size -- shared axes and fixed
aspects don't work well together (and manually resizing the figure shows
that the aspect is indeed not fixed). In fact, the
scatter_hist_locatable_axes example explicitly states that the advantage
of using axes_grid1 is to allow the aspect to be fixed.

I realized that one can also use inset_axes to position the marginals
axes relative to the main axes and support a fixed aspect ratio for
the main axes. Perhaps this can be considered as a slight API abuse,
but I think this is a solution for a real limitation; the question is
whether we want to promote this use?

PR Summary

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • 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).

@story645
Copy link
Member

story645 commented Oct 5, 2021

it is a valid way/I abuse inset axes a lot/I think it's okay since you're not removing the other one ways to do it - though I wonder then if there should be a top line explicit

  • pro
  • con

For each method so it doesn't read like 5 ways to do the same thing.

@anntzer
Copy link
Contributor Author

anntzer commented Oct 14, 2021

Reworded.

Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

Looks great to me other than a couple of minor suggestions below. Thanks!

Comment on lines 87 to 89
# Despite its name, `~.Axes.inset_axes` can also be used to position marginals
# *outside* the main axes. The advantage of doing so is that the aspect ratio
# of the main axes can be fixed, regardless of the figure size.
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
# Despite its name, `~.Axes.inset_axes` can also be used to position marginals
# *outside* the main axes. The advantage of doing so is that the aspect ratio
# of the main axes can be fixed, regardless of the figure size.
# `~.Axes.inset_axes` can be used to position marginals
# *outside* the main axes. The advantage of doing so is that the aspect ratio
# of the main axes can be fixed, and the marginals will always be drawn relative
# to the position of the axes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

Comment on lines +103 to +104
ax_histx = ax.inset_axes([0, 1.05, 1, 0.25], sharex=ax)
ax_histy = ax.inset_axes([1.05, 0, 0.25, 1], sharey=ax)
Copy link
Member

Choose a reason for hiding this comment

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

You uses 2/7 before = 0.285. Is there a reason to use 0.25 here? Probably insignificant, but maybe change them to be the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

switched to 4:1 everywhere.

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.

style nits, can be merged w/ or w/o changes


An alternative method to produce a similar figure using the ``axes_grid1``
toolkit is shown in the
:doc:`/gallery/axes_grid1/scatter_hist_locatable_axes` example.
:doc:`/gallery/axes_grid1/scatter_hist_locatable_axes` example. Finally, it is
also possible to simply position all axes in absolute coordinates using
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
also possible to simply position all axes in absolute coordinates using
also possible to position all axes in absolute coordinates using


* the axes positions are defined in terms of rectangles in figure coordinates
* the axes positions are defined via a gridspec
* the axes positions are defined via a gridspec;
Copy link
Member

Choose a reason for hiding this comment

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

maybe use

.. contents::
   :local:

to have links.

@@ -121,5 +119,6 @@ def scatter_hist(x, y, ax, ax_histx, ax_histy):
# - `matplotlib.figure.Figure.add_axes`
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
# - `matplotlib.figure.Figure.add_axes`

not used anymore.

Currently, the scatter_hist example needs to force the main axes to be
square by carefully adjusting the figure size -- shared axes and fixed
aspects don't work well together (and manually resizing the figure shows
that the aspect is indeed not fixed).  In fact, the
scatter_hist_locatable_axes example explicitly states that the advantage
of using axes_grid1 is to allow the aspect to be fixed.

I realized that one can also use inset_axes to position the marginals
axes relative to the main axes *and* support a fixed aspect ratio for
the main axes.  Perhaps this can be considered as a slight API abuse,
but I think this is a solution for a real limitation; the question is
whether we want to promote this use?
@anntzer
Copy link
Contributor Author

anntzer commented Oct 16, 2021

thanks, all comments handled

@story645 story645 merged commit a9bba75 into matplotlib:master Oct 16, 2021
@anntzer anntzer deleted the ia branch October 17, 2021 07:15
@QuLogic QuLogic added this to the v3.6.0 milestone Oct 18, 2021
tacaswell pushed a commit that referenced this pull request Oct 20, 2021
Demonstrate inset_axes in scatter_hist example.
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.

5 participants