Skip to content

MNT: update to more reliable method of creating legends#1

Merged
benlaken merged 1 commit intobenlaken:masterfrom
tacaswell:fix_legend
Oct 25, 2017
Merged

MNT: update to more reliable method of creating legends#1
benlaken merged 1 commit intobenlaken:masterfrom
tacaswell:fix_legend

Conversation

@tacaswell
Copy link
Copy Markdown
Contributor

This fixes a bug identified by @fperez

When calling ax.legend with one arg the list of strings is zipped
with the available artists in the Axes. This is brittle because it
assumes the contents and order of this list. In Matplotlib 1.5.1 we
added a legend handler for the artists that are used to draw the
fill_between regions which caused them to be included in the list of
artists which will go in the legend and due to internal details about
how Matplotlib store's the children artists of the Axes the
fill_between artists are listed before the errorbar artists and thus
the legends end up shifted.

The primary change in this commit is to pass the correct label into
the plotting calls via the label= kwarg and to call legend with no
args.

This fixes a bug identified by @fperez

When calling `ax.legend` with one arg the list of strings is zipped
with the available artists in the Axes.  This is brittle because it
assumes the contents and order of this list. In Matplotlib 1.5.1 we
added a legend handler for the artists that are used to draw the
fill_between regions which caused them to be included in the list of
artists which will go in the legend and due to internal details about
how Matplotlib store's the children artists of the Axes the
fill_between artists are listed before the errorbar artists and thus
the legends end up shifted.

The primary change in this commit is to pass the correct label into
the plotting calls via the `label=` kwarg and to call `legend` with no
args.
@benlaken
Copy link
Copy Markdown
Owner

benlaken commented Oct 9, 2017

Thanks for the PR - I will review this shortly!

@benlaken
Copy link
Copy Markdown
Owner

benlaken commented Oct 9, 2017

Hi Thomas, any chance you could share with me the output of pip freeze for your environment where you have this project running? Looks like I didn't have very good project management practices back when I wrote this, and neglected to include a requirements.txt file with frozen package versions.

@fperez
Copy link
Copy Markdown
Contributor

fperez commented Oct 12, 2017

@benlaken, thanks so much for making your research openly available, and in terms of reproducibility you're already doing better than the vast majority of the scientific community!

For context, the reason we found out about this, was b/c I used two of your papers as a homework and class project in my course on Reproducible and Collaborative Data Science at UC Berkeley. I hope you don't mind :)

For the first homework, the students had to practice with replicating your monsoon rainfall notebook. This meant downloading and being able to run it again, via github, working as a team.

Then for the project, they worked with this (European_wind) repo, and there they had to pretty much figure out the things you mention above: wrap the project in a Makefile along with an environment.yml (we're using conda envs, but same idea as pip freeze), while figuring out the versions you'd used, etc.

It was great to show this very page today during class, while we were discussing the project (their deadline was last night), and for them to see how the author of the paper they were working on was responding so kindly and openly, while identifying the same issues they were working on. I couldn't have timed it better if I'd tried :)

Once we're done grading, happy to send your way the environment.yml and Makefile, if you'd like to add them to the repo to make it a bit easier in the future...

Many thanks again! Open science rocks :)

@benlaken
Copy link
Copy Markdown
Owner

Thanks @fperez for the kind words - means a lot coming from you. I am also very happy to hear that my work has been useful on your course. You have some lucky students: I wish I had a similar course when I was studying!

Please do send a PR with the fixes and I will merge to Master - if it is useful for you, I can also leave a branch in its current state?

And indeed, rock-on Open Science! 🎸 💥

@benlaken benlaken merged commit 96aba87 into benlaken:master Oct 25, 2017
@benlaken
Copy link
Copy Markdown
Owner

Thanks Thomas - that was much appreciated! 👍

@tacaswell tacaswell deleted the fix_legend branch October 25, 2017 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants