Replace numeric loc by position string #11174

timhoffm · 2018-05-05T19:34:05Z

PR Summary

For better readability use legend(loc='upper right') instead of legend(loc=1)

The numbers will remain valid but won't be used anywhere in the docs and only very rarely in internal code.

jklymak · 2018-05-05T20:15:52Z

No doubt I have 🧠 damage from using matlab for too many years but I find the numbers easier to remember than the strings (at least for the corners). Is it “top” or “upper”, “bottom” or “lower”, “Center”, “centre”, or “middle”? I’d have to look it up. If we go string-only (even in the docs) I think the strings should be more forgiving and perhaps have short forms. “Ur” “lr” etc.

timhoffm · 2018-05-05T20:26:21Z

I won't let matlab knowledge count as an argument 😄 .

Readability counts: If someone reads code, 'upper right' is always better than 1. Everyone knows what 'upper right' means. In this case you don't have to worry about 'upper' vs. 'top'.
If you write code yourself and know the numbers, you can just continue to use them.
If you are new to matplotlib, you have to learn a convention anyhow. Then, I presume it's still easier to learn that it's upper/center/lower than to learn the numbers.

Additionally, the numbers don't even have a reasonable order:

2  9 1
6 10 7
3  8 4

Also, what's 'right' (5)? - Seems identical to 'center right' (7).

I'm -0.5 on more forgiving and short forms. While it may be easier to write, it reduces readability again and I don't want to a diversity of accepted arguments. Also keep in mind, that positioning the legend is something you do to polish your plot. So, loc is rather not used for a quick interactive plot, but when you want to create something nice, for which you probably want to keep the code. In particular then, I favor readability over saving a few characters.

efiring · 2018-05-05T20:45:43Z

An alternative is compass notation: N, E, S, W, NE, NW, etc. Easy to remember, concise.

timhoffm · 2018-05-05T20:51:34Z

Compass notation is currently used for anchors but not for legend locs. So this would be an API change, which would require a separate discussion.

jklymak · 2018-05-05T20:54:03Z

I won't let matlab knowledge count as an argument

Well, I don't 100% disagree with you, but... I don't think matplotlib would be in the position its in today if it hadn't hewed closely to Matlab: a) because Matlab actually is quite good in general, particularly with regards to API, particularly as they have a lot of smart people working on it for the last 30+ y. b) a lot of people come here from Matlab. So, while I don't think we should duplicate all of Matlab's API choices, its not a terrible starting spot....

I think compass notation in addition to the existing strings is a great idea.

anntzer · 2018-05-05T22:13:49Z

... but MATLAB doesn't even document numbers as locations? https://www.mathworks.com/help/matlab/ref/legend.html#bt6r30y They use compass notation, which I agree is a decent option.

ImportanceOfBeingErnest · 2018-05-05T22:16:03Z

While I do not think the number notation should vanish from the legend docstring (or other docs that use this notation), this PR seems to be concerned with the examples and tutorials only. Because such examples contain code that is to be read by people, it is useful to make it as understandable as possible. Using a clear notation helps to establish a connection between the code and the output, i.e. if the code reads legend(loc="upper left") and the legend appears in the upper left corner this is a very much expected result.

jklymak · 2018-05-05T22:18:26Z

Yes, as long as the docstring still has the locations thats fine.

jklymak

I didn't check every entry to see if the loc numbers -> string exactly, but this looks good.

jklymak · 2018-05-05T22:23:47Z

... but MATLAB doesn't even document numbers as locations?

Well, its been a while since I last used it 😉

http://matlab.izmiran.ru/help/techdoc/ref/legend.html


Obsolete Specifier | Location in Axes
-- | --
-1 | outside axes on right side
0 | inside axes
1 | upper right corner of axes
2 | upper left corner of axes
3 | lower left corner of axes
4 | lower right corner of axes

anntzer · 2018-05-05T22:27:23Z

So now MATLAB docs live on shady Russian websites? :p (sorry to any Russian I'm offending here).

dstansby

👍 @timhoffm feel free to either use my suggestions or merge this yourself if you don't want to

dstansby · 2018-05-06T20:08:49Z

doc/users/prev_whats_new/whats_new_2.1.0.rst

@@ -261,11 +261,11 @@ rectangle for the size bar.

    fig, ax = plt.subplots(figsize=(3, 3))

-    bar0 = AnchoredSizeBar(ax.transData, 0.3, 'unfilled', loc=3, frameon=False,
-                           size_vertical=0.05, fill_bar=False)
+    bar0 = AnchoredSizeBar(ax.transData, 0.3, 'unfilled', loc='lower left',


I wonder whether we should really change old what's new entries at all?

In this case, I'm in favor of changing. 'lower left' was already available at that time and should have been preferred to the number. We don't change any semantics but make the example more readable.

dstansby · 2018-05-06T20:10:16Z

lib/matplotlib/tests/test_legend.py

@@ -50,7 +51,7 @@ def test_legend_auto2():
    x = np.arange(100)
    b1 = ax.bar(x, x, align='edge', color='m')
    b2 = ax.bar(x, x[::-1], align='edge', color='g')
-    ax.legend([b1[0], b2[0]], ['up', 'down'], loc=0)
+    ax.legend([b1[0], b2[0]], ['up', 'down'], loc='best')


loc='best' is default, so maybe could be omitted altogether?

Yes. I've removed them.

jklymak · 2018-05-09T03:53:14Z

lib/matplotlib/tests/test_legend.py

@@ -39,7 +40,7 @@ def test_legend_auto1():
    x = np.arange(100)
    ax.plot(x, 50 - x, 'o', label='y=1')
    ax.plot(x, x - 50, 'o', label='y=-1')
-    ax.legend(loc=0)
+    ax.legend()


Tests don't like this...

Interesting. Apparently, the legend is placed 'upper right' when no argument is given. Do tests have defaults different from the standard matplotlib defaults?

I thought the default was 'best' instead of 'upper right'?

maybe style="mpl20"? Otherwise I think it uses classic... But its for a test, so maybe being explicit is better than relying on the style choice?

Ah, so the tests are still classic.

It's a bit unintuitive to have classic tests, because the code will behave different from more recent (v2) expectations. I see the point of not wanting to update all the tests just for the style though. Probably we should convert tests to mpl20 whenever we have to update the image anyway. That way, we'll get tests with mpl20 behavior in the long run without any additional overhead.

For now, I've put the explicit loc='best' back in.

efiring · 2018-05-09T18:07:08Z

On 2018/05/09 7:27 AM, Tim Hoffmann wrote: Probably we should convert tests to mpl20 whenever we have to update the image anyway. That way, we'll get tests with mpl20 behavior in the long run without any additional overhead.

Yes, that is the intended strategy.

timhoffm added this to the v3.0 milestone May 5, 2018

Replace numeric loc by position string

c6ce3f6

timhoffm force-pushed the legend-loc-strings branch from 6ea50c8 to c6ce3f6 Compare May 5, 2018 20:34

jklymak approved these changes May 5, 2018

View reviewed changes

dstansby approved these changes May 6, 2018

View reviewed changes

dstansby added the Documentation label May 6, 2018

jklymak reviewed May 9, 2018

View reviewed changes

Remove unnecessary explicit default loc='best' in legend()

e62a5ac

timhoffm force-pushed the legend-loc-strings branch from 0d0bde4 to e62a5ac Compare May 9, 2018 17:19

jklymak merged commit 3fd6919 into matplotlib:master May 9, 2018

timhoffm deleted the legend-loc-strings branch May 23, 2018 09:19

ImportanceOfBeingErnest mentioned this pull request Oct 30, 2018

ENH: Introduce compass-notation for legend #12679

Draft

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace numeric loc by position string #11174

Replace numeric loc by position string #11174

timhoffm commented May 5, 2018 •

edited

Loading

jklymak commented May 5, 2018 •

edited

Loading

timhoffm commented May 5, 2018 •

edited

Loading

efiring commented May 5, 2018

timhoffm commented May 5, 2018

jklymak commented May 5, 2018

anntzer commented May 5, 2018

ImportanceOfBeingErnest commented May 5, 2018

jklymak commented May 5, 2018

jklymak left a comment

jklymak commented May 5, 2018

anntzer commented May 5, 2018

dstansby left a comment

dstansby May 6, 2018

timhoffm May 6, 2018

dstansby May 6, 2018

timhoffm May 6, 2018

jklymak May 9, 2018

timhoffm May 9, 2018

dstansby May 9, 2018

jklymak May 9, 2018

timhoffm May 9, 2018

efiring commented May 9, 2018 via email

Replace numeric loc by position string #11174

Replace numeric loc by position string #11174

Conversation

timhoffm commented May 5, 2018 • edited Loading

PR Summary

jklymak commented May 5, 2018 • edited Loading

timhoffm commented May 5, 2018 • edited Loading

efiring commented May 5, 2018

timhoffm commented May 5, 2018

jklymak commented May 5, 2018

anntzer commented May 5, 2018

ImportanceOfBeingErnest commented May 5, 2018

jklymak commented May 5, 2018

jklymak left a comment

Choose a reason for hiding this comment

jklymak commented May 5, 2018

anntzer commented May 5, 2018

dstansby left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

efiring commented May 9, 2018 via email

timhoffm commented May 5, 2018 •

edited

Loading

jklymak commented May 5, 2018 •

edited

Loading

timhoffm commented May 5, 2018 •

edited

Loading