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

Rework a bit axes addition. #13269

Merged
merged 1 commit into from
Feb 2, 2019
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jan 23, 2019

  • Let _make_twin_axes rely on Axes.add_axes, rather than replicating
    its logic with process_projection_requirements. This needs a little
    dance to force add_axes to always create a new axes instead of
    returning a previously existing one though.
  • Then process_projection_requirements becomes only used by
    Figure.add_axes and Figure.add_subplot, so make it a private Figure
    method.
  • Cut-and-paste Figure._make_key to move it next to
    Figure._process_projection_requirements, which is itself next to
    add_axes and add_subplot. (Note that github shows this as add_artist
    moving above _make_key, rather than the other way round...)
  • Factor out some more common code between add_axes and add_subplot.

Preliminary work towards fixing #12513 (the end goal being to move the reactivation logic into pyplot).

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

sentinel = object()
real_label = kwargs.pop("label", sentinel)
twin = self.figure.add_subplot(
self.get_subplotspec(), *args, label=sentinel, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

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

Is it safe to set label to an object instance? According to the docstring it must be a str. Even if it works now, it might cause problems with future changes. Maybe use something like str(uuid.uuid4()) instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

- Let _make_twin_axes rely on Axes.add_axes, rather than replicating
  its logic with process_projection_requirements.  This needs a little
  dance to force add_axes to always create a new axes instead of
  returning a previously existing one though.
- Then process_projection_requirements becomes only used by
  Figure.add_axes and Figure.add_subplot, so make it a private Figure
  method.
- Cut-and-paste Figure._make_key to move it next to
  Figure._process_projection_requirements, which is itself next to
  add_axes and add_subplot.
- Factor out some more common code between add_axes and add_subplot.
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Anybody can merge after CI pass.

@jklymak jklymak merged commit b9c2b26 into matplotlib:master Feb 2, 2019
@anntzer anntzer deleted the axesaddition branch February 2, 2019 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants