Improve handling of shared axes with specified aspect ratio #10033
Conversation
4a5fe08 to
2f6147f
Compare
|
I think this is ready for people to try out. |
import matplotlib.pyplot as plt
import numpy as np
fig, ax = plt.subplots()
ax2 = ax.twinx()
ax.set_aspect(2)
pcm = ax.pcolormesh(np.random.rand(20,20))
fig.colorbar(pcm, ax = ax)
plt.show()used to give
Now gives
Seems better to me. OTOH, I don't understand why the ylim is not 0-20, and the xlim made larger to satisfy the aspect ratio arg, rather than the other way around. But maybe thats a different PR. There appears to be a bunch of stuff to do w/ gridlines in this PR; was it related? What is that trying to address? I still find it a tad annoying that import matplotlib.pyplot as plt
import numpy as np
fig, axs = plt.subplots(2, 1, sharex=True)
axs[0].set_aspect(3.)
plt.show()gives me this:
instead of this:
Removing the xticklabels in |
|
@jklymak, your first point is a demonstration of how keeping track of twinning, and keeping twinned boxes identical, fixes a long-standing bug. The answer to your question about which limits get adjusted is that we are severely limited by a fundamental aspect of the present implementation: apply_aspect is run inside each Axes.draw(). It can look to see whether any axes are shared or twinned, but it doesn't know anything about the sequence in which the draw commands are executed. In the case of twinned x axes, both the x view limits and the x (and y) box dimensions must be identical for the two axes. This means it is safe for apply_aspect only to adjust the y view limits, because they are independent (provided y is not also shared.) The gridline stuff is not really related, but it was in the bigger PR from which this one was derived. It is simplifying and making more consistent the handling of rcParams and grid properties. Certainly it could be split out; I will do it if I have to, eventually, but it will chew up time that I would rather not spend on that, because the end result will be no different. (I admit I had forgotten about the gridline part, and only noticed it yesterday when I was reviewing the PR as a whole.) For your last example, you need to use the new kwarg that I added to set the aspect ratio in both axes: import matplotlib.pyplot as plt
import numpy as np
fig, axs = plt.subplots(2, 1, sharex=True)
axs[0].set_aspect(3, share=True)
plt.show()
|
|
Alternatively, one can set the adjustable to 'datalim': import matplotlib.pyplot as plt
import numpy as np
fig, axs = plt.subplots(2, 1, sharex=True)
axs[0].set_aspect(3, adjustable='datalim')
axs[0].plot([1,2,3])
axs[1].plot([3,2,1])
plt.show()
|
|
That all sounds reasonable to me, and I don't think should slow this PR. My larger-scale complaint with what "share" means could be dealt with separately: I think the x-limit should be the same and the adjusted width be the same in 'box' mode, but the aspect ratios need not be the same, though the I don't mind the gridline stuff being in there, if it makes your life easier. But for review purposes it might help to have some test code and an explanation of what you are fixing/enhancing. |
|
Maybe in the case of subplots with only one axis shared, the adjustable should be automatically set to 'datalim'. This would be less of a break with previous behavior. It can't be done in the general case of sharing because it would not work with both axes shared and with axes shared across figures. |
82a986a to
68cc2f3
Compare
|
I moved the gridline part to #10193 and squashed the rest. |
lib/matplotlib/axes/_base.py
Outdated
| # locator and formatter attributes | ||
| self.xaxis.major = self._sharex.xaxis.major | ||
| self.xaxis.minor = self._sharex.xaxis.minor | ||
| x0, x1 = self._sharex.get_xlim() | ||
| self.set_xlim(x0, x1, emit=False, auto=None) | ||
| self.xaxis._scale = mscale.scale_factory( | ||
| self._sharex.xaxis.get_scale(), self.xaxis) | ||
| self._sharex.xaxis.get_scale(), self.xaxis) |
There was a problem hiding this comment.
Isn't this new indentation 21 spaces? That seems wrong.
lib/matplotlib/axes/_base.py
Outdated
| @@ -1010,14 +1004,13 @@ def cla(self): | |||
| y0, y1 = self._sharey.get_ylim() | |||
| self.set_ylim(y0, y1, emit=False, auto=None) | |||
| self.yaxis._scale = mscale.scale_factory( | |||
| self._sharey.yaxis.get_scale(), self.yaxis) | |||
| self._sharey.yaxis.get_scale(), self.yaxis) | |||
There was a problem hiding this comment.
Those are accidental, and I will revert them.
lib/matplotlib/tests/test_axes.py
Outdated
| @@ -4429,6 +4429,57 @@ def test_empty_shared_subplots(): | |||
| assert y1 >= 6 | |||
|
|
|||
|
|
|||
| def test_shared_with_aspect(): | |||
There was a problem hiding this comment.
Any reason not to turn this into 3 separate tests? This is going to stop running at the first failed assert, which seems like a bad thing when stuff breaks.
lib/matplotlib/axes/_base.py
Outdated
| @@ -4195,9 +4226,9 @@ def twiny(self): | |||
| return ax2 | |||
|
|
|||
| def get_shared_x_axes(self): | |||
| """Return a copy of the shared axes Grouper object for x axes.""" | |||
| 'Return a reference to the shared axes Grouper object for x axes' | |||
There was a problem hiding this comment.
This is actually less compliant with most of the tools I know, including the lack of period. Is this an intentional change?
There was a problem hiding this comment.
The change from "copy of" to "reference to" is a correction I made. I have reverted the other formatting changes, although my preference would actually be to use single double-quotes for these. I don't know why I used single-quotes.
There was a problem hiding this comment.
For the record, I like the single double- (or even single- !) quote too. I just know I've had to push away from those to make some checkers happy. (They're good to catch other problems.)
- 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.
07bd9a4 to
4d6448a
Compare
|
@dopplershift I restored the original docstring format (but not the incorrect content), and then rebased to solve a recently-introduced merge conflict. Mercifully, it was a simple one. |
|
Thanks a lot @efiring! |
| shared_y = self in self._shared_y_axes | ||
| # Not sure whether we need this check: | ||
| if shared_x and shared_y: | ||
| raise RuntimeError("adjustable='datalim' is not allowed when both" |
There was a problem hiding this comment.
Should it not be possible to share datalim with both axes?
There was a problem hiding this comment.
When the box dimensions are fixed, as is the case with "adjustable='datalim'", it is not possible to maintain a specified aspect ratio without being able to adjust at least one of the axes and have it stick. You can't do that when they are both shared.
There was a problem hiding this comment.
Let see if I get this straight. When adjustable is "box" they share both the datalim and the physical dim of the axis, but when adjustable is datalim it just it is the xlim or ylim that are shared?
What is the default adjustable when axes are created?
There was a problem hiding this comment.
Sharing applies only to datalim (in this context, but it is actually done via a reference to the same Axis, so locators and formatters as well as datalim are locked together). Twinning locks the box to be identical between the twins. The default adjustable is now box.
There was a problem hiding this comment.
Thus datalim/box are just used by the set_aspect.
With the example @stonebig and @mwaskom posted at #10585 twinning would become a new share group. That is the diagonal plots share y-axis. If you move up in (i,i) plot then i row will also move, but if you move up in (i,j) j not i then only that row except (i,i) will move.
This of course make sharing a bit harder with the new API (if it gets accepted), since sharing is transitive.
The only way to get the same result would be to listen to LocationEvent and break/create share
as one move between the axes. Or you need to open the make_twin API to the public.
PS. @efiring I fixed such that major and minor and copy over when share is broken in the new API.
|
I have come across this problem but it's not clear to me from the comments here how it was resolved. I have an For example, I want a tall |
|
Does this mean that with twinned axis, one cannot adjust the figure aspect ratio based on 'box' anymore? ax.set_aspect(abs((xright - xleft) / (ytop - ybottom)) * ratio, adjustable='box')Can I achieve this with twinned axis? |
|
"aspect" in matplotlib always refers to the data, not the axes box. Therefore setting the aspect for twinned or shared axes and letting the box be adjustable actually only makes sense when the scales are the same - or differ by an offset (as opposed to any other linear or nonlinear function). Matplotlib does not perform any check on this, so it disallows for If have the feeling that people often want to use the aspect to set the ratio of box height to box length. E.g. to achieve a square subplot. This is rather unrelated to how "aspect" is defined in matplotlib, but is sure a valid desire to have. Maybe something like |
Thats possible, but would require a good bit of thought as to how it would interact with the usual way of placing axes and the layout managers. |
This is an attempt to resurrect #8752. I stripped out the parts related to
cla()efficiency, thinking that had been handled by #8626, but now I see that the latter was reverted and does not appear to have been revived. I think the Axes initialization slowness problem was attacked from a different angle since then, but I haven't been able to find the PR. In any case, it is possible, but by no means certain, that something like the approach I used in 8752 should go back in. The idea is thatcla()is often too blunt an instrument; finer control over what is needed could be added via arguments to the function.In addition to clarifying the handling of aspect ratios with shared axes (see the
set_adjustabledocstring edit), this adds tracking of twinned axes, and keeps their positions in sync. This solves a bug in which adding a colorbar to a twinned axes previously would leave a mess--the box of the other twin was not being changed to match.ratio and adjustable in Axes within shared axis groups.