Simplify axes and axis clearing, sharing, and aspect ratio control by efiring · Pull Request #8752 · matplotlib/matplotlib

efiring · 2017-06-12T18:58:36Z

Most of the unnecessary calls to Axis.cla() have been removed.
Axis sharing works across figures.
The "box-forced" adjustable is no longer needed.
Sharing both axes requires the use of "box", not "datalim".
A new "share" kwarg triggers synchronized setting of aspect
ratio and adjustable in Axes within shared axis groups.
Added a test for axis sharing with aspect ratio setting.
Fixed and updated skew_rects test.

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/whats_new.rst if major new feature
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

- Most of the unnecessary calls to Axis.cla() have been removed. - Axis sharing works across figures. - The "box-forced" adjustable is no longer needed. - Sharing both axes requires the use of "box", not "datalim". - A new "share" kwarg triggers synchronized setting of aspect ratio and adjustable in Axes within shared axis groups. - Added a test for axis sharing with aspect ratio setting. - Fixed and updated skew_rects test.

tacaswell · 2017-06-12T19:38:22Z

attn @QuLogic and @astrofrog

efiring · 2017-06-13T00:12:55Z

Odd failure ("image file not readable") on the doc build:

Warning, treated as error:

/home/travis/build/matplotlib/matplotlib/doc/tutorials/01_introductory/sample_plots.rst::image file not readable: tutorials/01_introductory/../../gallery/lines_bars_and_markers/images/sphx_glr_fill_001.png

dstansby · 2017-06-13T18:06:20Z

Looks like some PEP8 errors too https://travis-ci.org/matplotlib/matplotlib/jobs/242143280#L2558

QuLogic · 2017-06-13T19:20:53Z

Some of these changes are similar to #8626, though perhaps with slightly different triggers.

QuLogic

I don't really know much about the sharing aspects; I looked mostly at the axis bits.

QuLogic · 2017-06-13T19:21:58Z

lib/matplotlib/axes/_base.py

+
+        if clear_axis:
+            self.xaxis.cla(shared_x)
+            self.yaxis.cla(shared_y)            #


Errant comment character.

QuLogic · 2017-06-13T19:24:49Z

lib/matplotlib/axes/_base.py

            self.yaxis.set_visible(yaxis_visible)
            self.patch.set_visible(patch_visible)

+        # It is not clear to me (EF) why this reset is needed here, but


Axes, Axis, Spine, and Tick are all inexplicably intertwined. I've been trying to disconnect them (e.g., #8678), but there's always one last bug every time...

QuLogic · 2017-06-13T19:27:33Z

lib/matplotlib/axis.py

+        self._grid_linewidth = (rcParams['grid.linewidth']
+                                if grid_linewidth is None else grid_linewidth)
+        self._grid_alpha = (rcParams['grid.alpha']
+                                if grid_alpha is None else grid_alpha)


Match opening parenthesis?

QuLogic · 2017-06-13T19:30:25Z

lib/matplotlib/axis.py

-        self.set_label_text('')
-        self._set_artist_props(self.label)
+        self.label.set_text('')  # self.set_label_text would change isDefault_
+        self._set_artist_props(self.label)  # sets figure; needed here?


Seems like it should be in __init__.

Actually it is already in _get_label, so I don't think we need it in cla or init.

QuLogic · 2017-06-13T19:31:27Z

lib/matplotlib/axis.py

+
        if reset:
            self.reset_ticks()
+        # Is this "else" correct? Shouldn't kwargs be applied after a reset?


reset_ticks deletes all ticks; they'll get recreated with the updated parameters from the dictionary.

OK, comment has been deleted.

QuLogic · 2017-06-13T19:32:44Z

lib/matplotlib/scale.py

-        axis.set_minor_locator(NullLocator())
        axis.set_minor_formatter(NullFormatter())
+        # update the minor locator for x and y axis based on rcParams
+        if (rcParams['xtick.minor.visible']):


Unnecessary parentheses.

efiring · 2017-06-15T21:06:02Z

@QuLogic, thank you for the review. I have addressed the minor points. I'm sorry for the overlap with your work; I haven't been keeping up with existing PRs.

This is required for consistency with the present grid() documentation and behavior, but the list of allowed properties should be trimmed.

efiring · 2017-06-27T11:56:24Z

This would close #2790; the test case described at the top of that issue works correctly.

tacaswell · 2017-07-02T22:11:17Z

I do not think forcing shared axes to 'box' is right

import matplotlib.pyplot as plt
fig, ax = plt.subplots()
ax2 = ax.twinx()
ax.set_aspect(5)

gives

on this branch.

This is to allow twinx and twiny to work as expected.

efiring · 2017-07-03T00:31:20Z

@tacaswell, yes, the twinx situation has to be handled differently. This arose recently in another issue as well. Twinx needs to ensure that the two axes have the same position, even if the position of one or the other is changed by any means.

jklymak · 2017-12-12T22:28:10Z

Pinging on the status of this PR or if there is any way I can help. I've assiduously avoided the aspect code in #9082 (layout manager) but it'll come up at some point. Knowing what the API is and what the current code is meant to do would be great...

efiring · 2018-06-11T00:24:50Z

Parts of this have come in via other PRs. The part that hasn't--the reduction of unnecessary clearing--will require a fresh look. Closing.

tacaswell added this to the 2.1 (next point release) milestone Jun 12, 2017

Fix PEP8 errors

2de8089

QuLogic reviewed Jun 13, 2017

View reviewed changes

Minor fixups from qlogic review

748c759

efiring added 3 commits June 15, 2017 17:50

Single extra space removed to placate PEP 8.

bff7e12

Handle all Line2D properties for gridlines.

f493ac3

This is required for consistency with the present grid() documentation and behavior, but the list of allowed properties should be trimmed.

PEP 8 again

d575d60

API: re-allow 'datalim' as a valid adjustable with shared axes

7dba686

This is to allow twinx and twiny to work as expected.

tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Aug 29, 2017

efiring mentioned this pull request Sep 1, 2017

ENH: add two new modes for 'adjustable' option #8700

Closed

6 tasks

This was referenced Nov 8, 2017

Axes.__init__ speedup #8626

Merged

Axes instantiation is very slow #6664

Closed

efiring mentioned this pull request Dec 4, 2017

Share and unshare axes after creation. #9923

Closed

6 tasks

efiring mentioned this pull request Dec 17, 2017

Improve handling of shared axes with specified aspect ratio #10033

Merged

efiring mentioned this pull request Jan 8, 2018

Handle Tick gridline properties like other Tick properties #10193

Merged

6 tasks

jklymak modified the milestones: needs sorting, v3.0 May 17, 2018

jklymak added the status: needs rebase label May 17, 2018

efiring closed this Jun 11, 2018

dstansby removed this from the v3.0 milestone Jun 11, 2018

Uh oh!

Conversation

efiring commented Jun 12, 2017

PR Summary

PR Checklist

Uh oh!

tacaswell commented Jun 12, 2017

Uh oh!

efiring commented Jun 13, 2017

Uh oh!

dstansby commented Jun 13, 2017

Uh oh!

QuLogic commented Jun 13, 2017

Uh oh!

QuLogic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efiring commented Jun 15, 2017

Uh oh!

efiring commented Jun 27, 2017

Uh oh!

tacaswell commented Jul 2, 2017

Uh oh!

efiring commented Jul 3, 2017

Uh oh!

jklymak commented Dec 12, 2017

Uh oh!

efiring commented Jun 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants