Enable set_{x|y|}label(loc={'left'|'right'|'center'}...) #15974

andrzejnovak · 2019-12-19T00:28:14Z

PR Summary

Addressed #15839 by adding functionality analogous to axes.set_title(loc="") for x/y axis and cbar axis as well as new rcParam to have a default config

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

jklymak · 2019-12-19T01:01:09Z

Thanks a lot for the PR! However, I think we should make sure that there is some consensus on this before you put too much effort into it.

I'm -0.5, because I'm not really sure how many people want to do this, or should, and that making it available is just adding to our already impressive kwarg list and rcParams. Is there a field where putting an axis label somewhere other than the centre is common? Are there other packages that allow axis labels to be placed this way? Whats the prior art here?

andrzejnovak · 2019-12-19T01:08:36Z

It's the default in high energy physics.

anntzer · 2019-12-19T18:49:58Z

Can you provide some examples, e.g. links to papers?

matplotlibrc.template

lib/matplotlib/axes/_axes.py

anntzer · 2019-12-19T18:54:03Z

lib/matplotlib/axes/_axes.py

@@ -246,6 +256,15 @@ def set_ylabel(self, ylabel, fontdict=None, labelpad=None, **kwargs):
        """
        if labelpad is not None:
            self.yaxis.labelpad = labelpad
+        if loc is None:
+            loc = rcParams['axis.titlelocation']
+        cbook._check_in_list(('left', 'center', 'right'), loc=loc)


can you screenshot how does that look?

So it's really top/bottom for y, which argues for the separate rcparams?

I think top/bottom is unnecessarily adding more keywords without helping the semantics. Should be still left/right even when aligning along the y axis

What do you mean? It's clearly at the top and the bottom?...
For example, if your y axis is oriented downwards, what do left and right correspond to?

What is a downward oriented y-axis ? The only case I can think of where it could be ambiguous is if someone flipped the text alignment such that one would title head right to read it as opposed to left as it usually is. Does that happen?

Ah, I see your point. I guess it was too subtle for me :)
I still think top/bottom are clearer (and these are names used elsewhere in the library, so we're not just making them up here), but won't block over that.

I think this also needs to be top/bottom. Even if you think this should be left/right I was surprised that you would say this was on the “right” side of the colorbar.

andrzejnovak · 2019-12-19T19:14:59Z

@anntzer See for example here:
https://twiki.cern.ch/twiki/bin/view/CMSPublic/PhysicsResultsHIG
https://arxiv.org/pdf/1207.7214.pdf
https://www.sciencedirect.com/science/article/pii/S0370269312008581

dopplershift · 2019-12-19T19:57:21Z

I'm ok with this as a feature. We already have this for title, so in that regards it might actually make the API easier to comprehend since things are more uniform. The code change is also straightforward enough. I just wish this didn't have to be changed in so many places...

anntzer · 2019-12-19T20:00:15Z

I think the feature is fine (... and likely to be merged faster than your drawstyle PR...), just needs polishing wrt rcParams and "top"/"bottom" vs "left"/"right" and docs?

tacaswell · 2019-12-20T02:19:52Z

Testing out what we talked about on this weeks call, I am going to nominate @dopplershift to be the liaison for getting this merged.

dopplershift · 2019-12-20T03:14:04Z

😱 😭

lib/matplotlib/axes/_axes.py

andrzejnovak · 2019-12-20T08:45:43Z

@timhoffm I was thinking about this, but since ha and x are lower level it would maybe make sense to me the other way around. I am not sure what's the best way to handle it, I could see either a) Ignore loc is either ha/x/y are supplied or b) raise an error if they are all supplied.

timhoffm · 2019-12-20T13:28:51Z

General comment on the design

The problem really is that loc and x/ horizontalalignment are interacting parameters on different abstraction levels. Generally, that's not a good pattern and I would avoid that. However, here I see the point of a loc parameter. The alternative using the same parameter on different levels, e.g. x='left' would be a bit confusing as well. Also there's precedence for loc for legend and title. Therefore I think loc is the best option. 👍

Managing conflicting parameters

But we need to handle the technically possible case that loc and x/ horizontalalignment are present. IMHO the best way is a hard failure with an exception. Having both is an incompatible specification. The user should just not do that and be made aware of it. We cannot know what he intended with that combination of parameters ("In the face of ambiguity, refuse the temptation to guess").

Behavior difference between `set_xlabel()` and `set_title()`

We should be aware that in this implementation set_xlabel(, loc=...) moves the label, but set_title(, loc=...) activates separate titles. This is different behavior and generally to be avoided. However I think the implementation is nevertheless right as is. Having multiple labels would be excessive (actually, I'd choose that behavior for title as well if I was able to design that API anew). Plus, going from one moving label to multiple labels later is almost non-breaking. The only break would occur if people would call set_xlabel() twice with different locs, but that is not a likely thing to happen. 👍

andrzejnovak · 2019-12-20T13:58:45Z

I'm fine with having a hard error as well.

Indeed it was not obvious where the changes should go, but it seemed the actual Text object wouldn't have the corresponding y/x axis differentiation...

The set_title behaviour is even more confusing imho. Consider:

fig, ax = plt.subplots()
ax.set_title("AA", loc='left')
ax.set_title("BB", loc='right', x=0)
ax.set_title("CC", loc='center', x=0, ha='left')
plt.show()

anntzer · 2019-12-20T14:00:21Z

We could perhaps try to deprecate the triple-title system (and support just one)... and see if anyone complains.

jklymak · 2019-12-20T14:52:10Z

Triple title is useful for a) b)c) subplot labelling with a centred title.

anntzer · 2019-12-20T14:55:25Z

Not sure what you exactly mean, but I guess that works only if you have exactly three (columns of) subplots?

jklymak · 2019-12-20T15:06:58Z

No. You put the panel label on the left “a)” often w a different font face, and the title in the centre.

anntzer · 2019-12-20T15:14:56Z

Ah, I thought you meant specifically a) (left) b) (center) c) (right) :)
Wouldn't your usecase be better served by #15771 (if that gets in)?

timhoffm · 2019-12-20T16:58:37Z

I wouldn't touch set_title for now, it's sort of ok-ish. But let's not get distracted that'd be for another PR anyway.

I'm just saying, even though there are some discrepancies to existing API, i think this PR is going the right direction.

doc/users/next_whats_new/2019-12-20-set_xy_position

examples/subplots_axes_and_figures/axis_labels_demo.py

lib/matplotlib/axes/_axes.py

anntzer · 2019-12-20T16:59:16Z

Agreed re: title.

dopplershift · 2019-12-21T02:12:02Z

There was a discussion before about changing the rcparam to axis.labellocation. I don't see anywhere where we resolved it. Since we're talking about labels, labellocation seems more appropriate than titlelocation. Thoughts?

jklymak

Looks like there is consensus this is useful, so that is great. But we don’t call the axis labels “titles”. We call them “labels” so please use that term...

lib/matplotlib/axes/_axes.py

jklymak · 2019-12-21T03:15:34Z

lib/matplotlib/axes/_axes.py

@@ -246,6 +256,15 @@ def set_ylabel(self, ylabel, fontdict=None, labelpad=None, **kwargs):
        """
        if labelpad is not None:
            self.yaxis.labelpad = labelpad
+        if loc is None:
+            loc = rcParams['axis.titlelocation']
+        cbook._check_in_list(('left', 'center', 'right'), loc=loc)


I think this also needs to be top/bottom. Even if you think this should be left/right I was surprised that you would say this was on the “right” side of the colorbar.

lib/matplotlib/tests/test_axes.py

andrzejnovak · 2019-12-21T11:00:44Z

@jklymak I feel like top/bottom replacement is unnecessary.

Prior to this PR the effect has to be achieved by setting x/y and ha='right'/'left' regardless of whether it's x os y axis, so it more consistent
It refers to the text alignment along the axis and in the majority of cases the text is written such that if you turn your head to read it, there's a clear 'left'/'right' direction
It would be an additional complication when plotting colorbar - switching the orientation would then also require to switch the loc keyword

timhoffm · 2019-12-21T15:12:59Z

Intuitively, I'd expect top/center/bottom for the yaxis. Everybody will understand that, whereas left/center/right needs some explanation.

Also semantically, this is the high-level approach only specifying the location. In contrast to alignment it does not have a directional/orientational character.

AFAIK switching the orientation later is not possible. So it's only a matter of parameter checking/translation when creating the colorbar. Let's also not forget that this is an advanced parameter to tune your plot. If people switch the colorbar direction, it's unclear if a vertical bottom label should be translated to a horizontal left label. IMHO being more explicit and strict in this case is actually a plus.

andrzejnovak · 2019-12-21T15:26:05Z

Well actually semantically it is just two options - min/max along the direction of the axis whatever the orientation. Whereas an ensemble of top/bottom/right/left options hints at 4 different positions, which is not the case. But I do seem to be in the minority.

The code to do this for cbar would have to have the same exact logic too:

        if loc in ['right', 'top']:
            kwargs[_pos_xy] = 1.
            kwargs['horizontalalignment'] = 'right'
        elif loc in ['left', 'bottom']:
            kwargs[_pos_xy] = 0.
            kwargs['horizontalalignment'] = 'left'

lib/matplotlib/axes/_axes.py

lib/matplotlib/colorbar.py

andrzejnovak · 2019-12-31T09:42:39Z

@timhoffm That's not how verticalalignment works. ha/va are in relation the text orientation, they don't swap between x/y axis.

ax.set_ylabel('YLabel', va='top')

ax.set_ylabel('YLabel', va='bottom')

timhoffm · 2020-01-05T20:50:02Z

Changes should be squashed (either using a force-push or by the second reviewer through the GitHub UI when merging).

lib/matplotlib/rcsetup.py

anntzer · 2020-01-20T12:38:00Z

lib/matplotlib/rcsetup.py

@@ -988,7 +988,12 @@ def validate_webagg_address(s):
    raise ValueError("'webagg.address' is not a valid IP address")


-validate_axes_titlelocation = ValidateInStrings('axes.titlelocation', ['left', 'center', 'right'])
+_validate_axes_titlelocation = ValidateInStrings('axes.titlelocation',


Ah yes. Sorry I missed that; it'll need to start public for now (and get properly deprecated later).

anntzer · 2020-01-20T13:50:16Z

lib/matplotlib/axes/_axes.py

+        _protected_kw = ['x', 'horizontalalignment', 'ha']
+        if any([k in kwargs for k in _protected_kw]):
+            if loc is not None:
+                raise KeyError('Specifying *loc* is disallowed when any of '


This should be a TypeError (it's a basically a signature mismatch, which normally raise TypeErrors).

anntzer · 2020-01-20T13:50:46Z

lib/matplotlib/axes/_axes.py

+        _protected_kw = ['y', 'horizontalalignment', 'ha']
+        if any([k in kwargs for k in _protected_kw]):
+            if loc is not None:
+                raise KeyError('Specifying *loc* is disallowed when any of '


anntzer · 2020-01-20T14:55:03Z

looks like you messed up the rebase :(

andrzejnovak · 2020-01-20T15:04:30Z

looks like you messed up the rebase :(

indeed

lib/matplotlib/axes/_axes.py

anntzer · 2020-01-20T16:59:04Z

flake8 is still unhappy: https://travis-ci.com/matplotlib/matplotlib/jobs/277655865#L2387

anntzer

Anyone can merge after CI passes.

anntzer · 2020-01-20T21:02:11Z

Thanks for the contribution :)

anntzer reviewed Dec 19, 2019

View reviewed changes

matplotlibrc.template Outdated Show resolved Hide resolved

anntzer reviewed Dec 19, 2019

View reviewed changes

lib/matplotlib/axes/_axes.py Outdated Show resolved Hide resolved

anntzer reviewed Dec 19, 2019

View reviewed changes

tacaswell added this to the v3.3.0 milestone Dec 20, 2019

tacaswell assigned dopplershift Dec 20, 2019

timhoffm reviewed Dec 20, 2019

View reviewed changes

lib/matplotlib/axes/_axes.py Outdated Show resolved Hide resolved

lib/matplotlib/axes/_axes.py Outdated Show resolved Hide resolved

lib/matplotlib/axes/_axes.py Outdated Show resolved Hide resolved

timhoffm reviewed Dec 20, 2019

View reviewed changes

doc/users/next_whats_new/2019-12-20-set_xy_position Outdated Show resolved Hide resolved

examples/subplots_axes_and_figures/axis_labels_demo.py Outdated Show resolved Hide resolved

lib/matplotlib/axes/_axes.py Outdated Show resolved Hide resolved

jklymak reviewed Dec 21, 2019

View reviewed changes

timhoffm reviewed Dec 30, 2019

View reviewed changes

timhoffm approved these changes Jan 5, 2020

View reviewed changes

timhoffm added the New feature label Jan 5, 2020

andrzejnovak force-pushed the axtitle branch from 01163f8 to 9c33288 Compare January 6, 2020 09:54