Make legend handling more intuitive and consistent with `matplotlib` #30

lukashergt · 2019-07-14T22:45:02Z

Description

Legend handling is now more intuitive, i.e. can be done in the usual matplotlib fashion.

Make the link axes[x][x] = axes[x][x].twin such that the user has direct access to the diagonal axes that have actually been drawn in.
- axes[x][x].legend() now automatically makes a legend in the diagonal axes.
Create the legend proxy already in contour_plot_2d and pass it to ax.patches.
- axes[x][y].legend() now automatically makes a legend in the 2D 'kde' axes.
- No need for get_legend_proxy anymore, because matplotlib's own functions can handle this, now:
```
handles, labels = axes[x][y].get_legend_handles_labels()
leg = fig.legend(handles, labels)
```
- Flagged get_legend_proxy as deprecated.

Checklist:

I have performed a self-review of my own code
My code is PEP8 compliant (flake8 anesthetic tests)
My code contains compliant docstrings (pydocstyle --convention=numpy anesthetic)
New and existing unit tests pass locally with my changes (python -m pytest)
I have added tests that prove my fix is effective or that my feature works

* At the moment axes[x][x] are empty and axes[x][x].twin are the ones that have actually been drawn in. This is rather opaque for the user so I think it would make sense to link the diagonal axes to their twin: `axes[x][x] = axes[x][x].twin` - Now the user can call `axes[x][x].legend()` and gets matplotlib's automated legend behaviour for the diagonal plots. - Is the original axes needed for something? Then it might be worth saving it as `axes[x][x].orig = axes[x][x]` first... Changes to be committed: * modified: anesthetic/plot.py * modified: anesthetic/samples.py

* Contour plots are too complex in the scope of matplotlib for the purposes of legend handling. Here the contour plots typically only draw 2 levels making a simple Rectangle patch a suitable legend handle. * Added legend proxies for contour plots directly to their axes such that axes legends can now be drawn simply by calling: ``` axes[x][y].legend()` ``` * This also simplifies figure legends because now you can simply do: ``` handles, labels = axes[x][y].get_legend_handles_labels() fig.legend(handles, labels) ``` - `get_legend_handles_labels` is matplotlib's standard way of handling this sort of thing. - This makes the helper function `get_legend_proxy` redundant. Changes to be committed: * modified: anesthetic/plot.py

williamjameshandley

Could you also add a test that creates a legend using the new functionality?

anesthetic/plot.py

anesthetic/samples.py

codecov · 2019-07-15T08:23:52Z

Codecov Report

❗ No coverage uploaded for pull request base (master@eefab0a). Click here to learn what that means.
The diff coverage is 80%.

@@            Coverage Diff            @@
##             master      #30   +/-   ##
=========================================
  Coverage          ?   90.22%           
=========================================
  Files             ?       14           
  Lines             ?      992           
  Branches          ?        0           
=========================================
  Hits              ?      895           
  Misses            ?       97           
  Partials          ?        0

Impacted Files	Coverage Δ
anesthetic/samples.py	`90.39% <100%> (ø)`
anesthetic/plot.py	`90.09% <60%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eefab0a...7e1c137. Read the comment docs.

codecov · 2019-07-15T08:23:52Z

Codecov Report

Merging #30 into master will decrease coverage by 0.11%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
- Coverage   89.95%   89.84%   -0.12%     
==========================================
  Files          14       14              
  Lines         986      995       +9     
==========================================
+ Hits          887      894       +7     
- Misses         99      101       +2

Impacted Files	Coverage Δ
anesthetic/plot.py	`89.32% <83.33%> (-0.53%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eefab0a...8909e4f. Read the comment docs.

anesthetic/plot.py

The original axes are where ticks and labels are handled so should not be discarded in favour of the twin axes. Instead just pass the lines and patches from the twin on to the original axes for 2D plots.

lukashergt · 2019-07-15T11:02:10Z

Could you also add a test that creates a legend using the new functionality?

Added a test that checks the handles and labels. Is that what you had in mind?

* Remove zorder changes for contour plots. - Any legend placed in axes with contour plots got hidden behind the contours. - Not sure what the original reason for the zorder changes was. * Set axes in front of axes.twin. - Since legends are drawn on axes, they got hidden behind the lines and histograms which belonged to the twin axes and thus were in front. Changes to be committed: * modified: anesthetic/plot.py

…t to overplot contours from other models

williamjameshandley · 2019-07-15T12:33:27Z

Added a test that checks the handles and labels. Is that what you had in mind?

That looks great.

williamjameshandley · 2019-07-15T12:34:34Z

One further comment -- If I try calling fig.legend, it produces a large legend for every axis. Is this the expected/desired behaviour?

…tions and containers

lukashergt · 2019-07-15T13:36:45Z

One further comment -- If I try calling fig.legend, it produces a large legend for every axis. Is this the expected/desired behaviour?

Yes, that is the matplotlib behaviour. This happens when you call the figure legend instead of the axis legend. The figure contains all the axes and thus shows all the handles and labels from all the axes.

williamjameshandley · 2019-07-15T13:44:45Z

Yes, that is the matplotlib behaviour. This happens when you call the figure legend instead of the axis legend. The figure contains all the axes and thus shows all the handles and labels from all the axes.

So how should we advise users to implement a 'figure legend' in the natural way, namely producing a summary that is not a repeat for each sub-plot?

lukashergt · 2019-07-15T13:50:06Z

So how should we advise users to implement a 'figure legend' in the natural way, namely producing a summary that is not a repeat for each sub-plot?

See description at the top. I also put this in the deprecation warning for get_legend_proxy.

anesthetic/samples.py

anesthetic/plot.py

…_2d_plot to original axes

williamjameshandley · 2019-07-15T15:17:54Z

I'm trying to update the demo.py and demo.ipynb with your changes

prior = nested.set_beta(0)
fig, axes = prior.plot_2d(['ns','tau'])                                                          
nested.plot_2d(axes=axes)
handles, labels = axes['ns']['tau'].get_legend_handles_labels()
leg = fig.legend(handles, labels)

But no legend is produced (only an empty top-right box). The handles and labels lists are empty. Do you also get this behaviour?

lukashergt · 2019-07-15T15:28:12Z

I'm trying to update the demo.py and demo.ipynb with your changes

But no legend is produced (only an empty top-right box). The handles and labels lists are empty. Do you also get this behaviour?

Are you sure they are using this branch?

lukashergt · 2019-07-15T15:30:32Z

Never mind, you are not passing any labels. Do this:

prior = nested.set_beta(0)
fig, axes = prior.plot_2d(['x0','x1'], label='prior')                                                          
nested.plot_2d(axes=axes, label='posterior')
handles, labels = axes['x0']['x1'].get_legend_handles_labels()
leg = fig.legend(handles, labels)

williamjameshandley · 2019-07-15T15:40:24Z

Cool. That seems to have worked. I'll update the plot gallery once version 1.1 is out.

lukashergt · 2019-07-15T15:56:37Z

You don't care about PEP8 in the demos? (flake8 demo.py)

williamjameshandley · 2019-07-15T15:59:35Z

No, demo.py is for generating the ipython notebook using py2nb. flake8ing it is overkill.

williamjameshandley · 2019-07-15T16:00:17Z

(by all means propose a PR that fixes it)

Lukas Hergt added 4 commits July 14, 2019 22:04

small fixes for tests to pass

0ccf165

make deprecated and throw a warning

eebd3a6

lukashergt added the enhancement New feature or request label Jul 14, 2019

lukashergt added this to the 1.1 milestone Jul 14, 2019

lukashergt self-assigned this Jul 14, 2019

lukashergt mentioned this pull request Jul 14, 2019

Fix colorcycle #32

Merged

5 tasks

lukashergt requested a review from williamjameshandley July 15, 2019 00:02

williamjameshandley reviewed Jul 15, 2019

View reviewed changes

anesthetic/plot.py Show resolved Hide resolved

anesthetic/samples.py Outdated Show resolved Hide resolved

Merge branch 'master' into contourf_legend

7e1c137

lukashergt commented Jul 15, 2019

View reviewed changes

anesthetic/plot.py Show resolved Hide resolved

Lukas Hergt added 2 commits July 15, 2019 11:05

pass lines and patches from twin to twin instead of reassigning axes

9f00b05

The original axes are where ticks and labels are handled so should not be discarded in favour of the twin axes. Instead just pass the lines and patches from the twin on to the original axes for 2D plots.

add test for new legend behaviour

f623e05

Lukas Hergt added 2 commits July 15, 2019 12:13

set default zorder for contour AND contourf to 1, needed for lines no…

87eb3fb

…t to overplot contours from other models

pass complete list of handles from twin to original, i.e. also collec…

3af53bb

…tions and containers

williamjameshandley reviewed Jul 15, 2019

View reviewed changes

anesthetic/samples.py Outdated Show resolved Hide resolved

anesthetic/plot.py Show resolved Hide resolved

pass twin lines, patches, collections, and containers already in make…

f3986c4

…_2d_plot to original axes

updated demo scripts

8909e4f

williamjameshandley approved these changes Jul 15, 2019

View reviewed changes

williamjameshandley merged commit 1b4b3e5 into handley-lab:master Jul 15, 2019

lukashergt deleted the contourf_legend branch July 15, 2019 16:07

This was referenced Jul 17, 2019

Preliminary tricontour functionality #25

Closed

Automatic labelling #42

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make legend handling more intuitive and consistent with `matplotlib` #30

Make legend handling more intuitive and consistent with `matplotlib` #30

lukashergt commented Jul 14, 2019 •

edited

Loading

williamjameshandley left a comment

codecov bot commented Jul 15, 2019

codecov bot commented Jul 15, 2019 •

edited

Loading

lukashergt commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

lukashergt commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

lukashergt commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019 •

edited

Loading

lukashergt commented Jul 15, 2019

lukashergt commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

lukashergt commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

Make legend handling more intuitive and consistent with matplotlib #30

Make legend handling more intuitive and consistent with matplotlib #30

Conversation

lukashergt commented Jul 14, 2019 • edited Loading

Description

Checklist:

williamjameshandley left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 15, 2019

Codecov Report

codecov bot commented Jul 15, 2019 • edited Loading

Codecov Report

lukashergt commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

lukashergt commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

lukashergt commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019 • edited Loading

lukashergt commented Jul 15, 2019

lukashergt commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

lukashergt commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

williamjameshandley commented Jul 15, 2019

Make legend handling more intuitive and consistent with `matplotlib` #30

Make legend handling more intuitive and consistent with `matplotlib` #30

lukashergt commented Jul 14, 2019 •

edited

Loading

codecov bot commented Jul 15, 2019 •

edited

Loading

williamjameshandley commented Jul 15, 2019 •

edited

Loading