Skip to content

Work towards removing reuse-of-axes-on-collision. #10660

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
Mar 4, 2018

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Mar 3, 2018

Currently, Matplotlib reuses axes when add_axes() is called a second
time with the same arguments. This behavior is deprecated since 2.1.
(xref #9037)

However we forgot to deprecate the same behavior in gca(), so we can't
remove that behavior yet.

Also cleanup docstrings of Stack class. Also,
process_projection_requirements cannot modify the outer kwargs (because
**kwargs is always a copy), so remove the incorrect note regarding the
need for copies.

(milestoning as 3.0 but would actually be nice to at least have the deprecation in 2.2...)

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 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

@anntzer anntzer added this to the v3.0 milestone Mar 3, 2018
in the stack
"""

"""Raise element (present in the stack) to the stack top; return it."""
Copy link
Member

Choose a reason for hiding this comment

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

To me, this is a bit too shorted, so that is not easily to read. Alternative suggestion:
"Raise element o to the stack top and return it. The element must be present in the stack."

if self._pos > 0:
self._pos -= 1
return self()

def push(self, o):
"""
push object onto stack at current position - all elements
occurring later than the current position are discarded
Push element to stack at current position, discard all later elements.
Copy link
Member

Choose a reason for hiding this comment

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

Not sure what the policy with docstings is, but I'd like to have articles. Also I suggest to either use and or a period between the sentences. Reads more nicely.

@@ -1249,7 +1242,7 @@ def bubble(self, o):
return o

def remove(self, o):
'remove element *o* from the stack'
"""Remove element from the stack."""
Copy link
Member

@timhoffm timhoffm Mar 3, 2018

Choose a reason for hiding this comment

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

Either "Remove element o from the stack." or "Remove the element from the stack."

@@ -67,15 +67,11 @@ def get_projection_class(projection=None):

def process_projection_requirements(figure, *args, **kwargs):
"""
Handle the args/kwargs to for add_axes/add_subplot/gca,
returning::
Handle the args/kwargs to for add_axes/add_subplot/gca, returning::
Copy link
Member

Choose a reason for hiding this comment

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

"to/for" or just "for"

Currently, Matplotlib reuses axes when add_axes() is called a second
time with the same arguments.  This behavior is deprecated since 2.1.

However we forgot to deprecate the same behavior in gca(), so we can't
remove that behavior yet.

Also cleanup docstrings of Stack class.  Also,
process_projection_requirements cannot modify the outer kwargs (because
`**kwargs` is always a copy), so remove the incorrect note regarding the
need for copies.
@anntzer
Copy link
Contributor Author

anntzer commented Mar 3, 2018

reworked docstrings

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.

3 participants