new methods set_xlabel and set_ylabel in "class Figure" #1519

Closed
wants to merge 12 commits into
from

Conversation

Projects
None yet
6 participants
Contributor

montefra commented Nov 19, 2012

Very crude attempt to implement a figure-wide x and y label. I think that this can be useful for figures with multiple plots with same x and y axis.

As now the methods check for the absolute extrema of all the axes/subplots in the figure, and write the label as a text: xlabel below the lower axes and ylabel in vertical on the left.
In both cases the labels are centred with respect to the total area occupied by the axes/subplots

The current implementation has some (important) shortcoming:

  1. the methods must be called at the end
  2. they do not interact with tight_layout
  3. the offset from the axis must be set by hand and change according to the font size and the number of digits on the y axis

A way to solve this problem would be to create an empty axes enclosing all the other axes/subplots and to show only the x and y labels.
If anyone has pointers/suggestion on how to make those two methods more in the line of matplotlib and useful to the general user, please let me know. I'm happy to work on them

lib/matplotlib/figure.py
+ for information on how override and the optional args work
+
+ WARNING: as now it works only if used after all the axes/subplots
+ have been created and require fine tuning (there is no integration
@pelson

pelson Nov 20, 2012

Member

#1448 would provide this.

@montefra

montefra Nov 20, 2012

Contributor

Good to know
edit: Do I take out the warning, then?

@jenshnielsen

jenshnielsen Nov 20, 2012

Owner

Does it also fix it with tight_layout()? I thought that it only had to do with bbox_inches='tight'

@pelson

pelson Nov 20, 2012

Member

Ah - yes your right @jenshnielsen - I still read the two as the same thing (but obviously are completely different beasts...). Seems the warning should stay.

lib/matplotlib/figure.py
+ """
+ Call signature::
+
+ set_xlabel(xlabel, fontdict=None, labelpad=None, **kwargs)
@pelson

pelson Nov 20, 2012

Member

docstring is different to signature.

@montefra

montefra Nov 20, 2012

Contributor

Cut and paste error. Thanks for pointing out. I'll fix it as soon as I can.

Member

pelson commented Nov 20, 2012

This line (where figure is using get_axes) is the biggest problem for me. The functionality itself does sound useful - I wonder if it fits better as a method on Gridspec?

@ those with gridspec / subplot knowledge - what do you think about adding such a figure method?

Contributor

montefra commented Nov 20, 2012

@pelson : I'm aware that the implementation of the two methods is not such a beauty. I don't how to look for the outer edges of all the panels in a figure. If you have any better way to find the overall size of the axes/subplots, I'm happy to change it.
An possible way to improve is to be to a functionality in add_subplot/add_axes that records the first columns and last row axes (similar to what is done in matplotlib.axes.SubplotBase.label_outer) and then avoid the call of get_axes in set_[xy]label. Probably I should also save somewhere the lable text, to remove the old label if the method is called more than once.
About the gridspec/subplot: I think that figure-wide axes labels should accessible whatever is the method used to create the panels (axes, subplots, gridspec, ect), so they should associated to the figure hosting the plots.

Contributor

montefra commented Dec 5, 2012

[edited] I changed the way set_[xy]label works. Now add_axes or add_subplot call _set_bigextents, that updates a variable self.bigextents (a box containing all the axes/subplots in the figure). When calling set_[xy]label values in self.bigextents are used. If a label was already given, the old one is set to invisible and a new one is written.
I think that now is better than before.

@pelson : I think that now the implementation is much better (and I don't use anymore get_axes when setting the labels)

this should probably be substituted with self.xlabel.remove() but it looks like that the method is not implemented yet (the same in set_ylabel)

Contributor

montefra commented Dec 18, 2012

It looks like that Travis get confused by my commits (I rebased before pushing). I can build (python setup.py build) and produce a figure with subplots and figure-wide axis label without problem on my machine.
From Travis logs it look like it ends up somewhere between commits e92e15d and ed364c9, at it complains about non existing _rect_largest that I have renamed in the last commit (I'm sure that I tested my implementation without problems before and after the name change)
Do I have to worry about it?

Owner

mdboom commented Dec 18, 2012

It looks like you removed _rect_largest in ed364c9 but it is still being called from line 877 in figure.py. I think that latter line also needs to be updated.

Contributor

montefra commented Dec 18, 2012

me stupid. Thank for catching it @mdboom. I completely forgot that I added the function call there and I guess that pushing changes after midnight wasn't such a smart idea. Now Travis i happy (I think that I owe him a coffee 😄)

lib/matplotlib/figure.py
@@ -870,6 +874,7 @@ def add_subplot(self, *args, **kwargs):
if isinstance(ax, projection_class):
# the axes already existed, so set it as active & return
self.sca(ax)
+ self._set_bigextents( ax )
@WeatherGod

WeatherGod Dec 19, 2012

Member

PEP8: do not put spaces around these variables in a function call

lib/matplotlib/figure.py
@@ -807,6 +807,10 @@ def add_axes(self, *args, **kwargs):
self._axstack.add(key, a)
self.sca(a)
+
+ #update the bounding box containing all the axes
@WeatherGod

WeatherGod Dec 19, 2012

Member

Space after the comment symbol

lib/matplotlib/figure.py
return a
+ def _set_bigextents( self, ax ):
+ """Given an axis/subplot instance create or update a the extent
@WeatherGod

WeatherGod Dec 19, 2012

Member

PEP8: triple quotes for docstrings should be on its own line

lib/matplotlib/figure.py
return a
+ def _set_bigextents( self, ax ):
@WeatherGod

WeatherGod Dec 19, 2012

Member

PEP8, again with the extraneous spaces...

lib/matplotlib/figure.py
+ """
+ axextents = ax.get_position().extents #rectangle of the axis/subplot
+ try:
+ self.bigextents
@WeatherGod

WeatherGod Dec 19, 2012

Member

do hasattr(self, 'bigextents') instead... much cleaner. Also, wouldn't it be better to simply initialize it in the constructor?

lib/matplotlib/figure.py
+ self.xlabel.set_visible( False )
+ #write the xlabel as text below the lower axes. labelpad
+ #should leave enough space for the default xticklabels
+ self.xlabel = self.text( (self.bigextents[2]+self.bigextents[0])/2,
@WeatherGod

WeatherGod Dec 19, 2012

Member

PEP8: spaces around binary operators like '+' and '/'

lib/matplotlib/figure.py
@@ -1149,7 +1272,7 @@ def gca(self, **kwargs):
Return the current axes, creating one if necessary
The following kwargs are supported for ensuring the returned axes
- adheres to the given projection etc., and for axes creation if
+ dheres to the given projection etc., and for axes creation if
@WeatherGod

WeatherGod Dec 19, 2012

Member

Seem to have inadvertently deleted an 'a' here...

I figured that this initialization is better than something like self.bigextent = [1.0, 1.0, 0, 0]

If set_[xy]label is called before adding any axes/subplots this will give an error.
Would be better to have a check for self.bigextents and throw an warning if it is None. If yes which one?

Contributor

montefra commented Dec 19, 2012

Is there a way to get the high of the xaxis labels and the width of the yaxis labels in figure coordinates?
If yes, would be nice to change the labelpad automatically when set_[xy]label is called. Now it can require some call of the methods to get the correct spacing of the labels.

Contributor

montefra commented Mar 18, 2013

today it came to me that I might move _set_bigextents(self, ax) to AxesStack so that I can update the bigextents when axes are added and removed. Does it make sense?

Contributor

leejjoon commented May 9, 2013

I think the current implementation is very limited. For example, the position of label does not change even if the position of axes changes. I think the better approach is to create a new artist (derived from Text) that calculate the bigextents and update its position, during the drawing time .

Contributor

montefra commented May 31, 2013

@leejjoon : sorry for not replying earlier. I know that is very limited: I have posted my code anyway to propose the idea and get feedback. I never thought of doing as you suggest. I'll give a try: when I have some time, I'll look into Text and try to find a good solution.

Member

pelson commented Jan 9, 2014

Closing this PR as it has stalled. I can't see any other objections other than mine about the added set_[xy]label methods, so I suspect in principle there is no reason that such functionality couldn't be added to the Figure class in the future if you wanted to revive/recreate this PR. Please feel free to do so, and either re-open this PR, or create a new one.

Cheers!

@pelson pelson closed this Jan 9, 2014

Contributor

montefra commented Jan 9, 2014

@pelson: thanks for reminding me of this PR 😄 when I have time/will I'll give it a try again. If I come up with a better implementation I'll come back

Member

pelson commented Jan 9, 2014

when I have time/will I'll give it a try again

I just encourage you to not despair - sometimes "no" to a feature is a good answer for the health of an open source project. On the other hand, if you were fixing a bug/extending the documentation I think we would all bite your hands off 😄

Contributor

montefra commented Jan 9, 2014

I'm not despairing, don't worry. Also because I cannot be that attached to something I haven't touched for such a long time.

On the other hand, if you were fixing a bug/extending the documentation I think we would all bite your hands off

you mean that you would do this if I try to fix bugs/extending the documentation or if it takes an year to do so? (I hope the second 😜 )

Member

pelson commented Jan 9, 2014

you mean that you would do this if I try to fix bugs/extending the documentation or if it takes an year to do so? (I hope the second 😜 )

Sorry, it is a turn of phrase not a threat 😄 - http://www.english-test.net/forum/ftopic650.html

Member

WeatherGod commented Jan 9, 2014

Must be a British thing... never heard of that phrase either.

Member

pelson commented Jan 9, 2014

Must be a British thing... never heard of that phrase either.

International collaboration is a wonderful thing 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment