Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add legend as dict for line plots with test #71

Merged
merged 3 commits into from
Jul 17, 2018

Conversation

gidden
Copy link
Member

@gidden gidden commented Jul 11, 2018

Please confirm that this PR has done the following:

  • Tests Added
  • Documentation Added
  • Description in RELEASE_NOTES.md Added

Adding to RELEASE_NOTES.md

Please add a single line in the release notes similar to the following:

- (#XX)[http://link-to-pr.com] Added feature which does something

pyam/plotting.py Outdated
Include a legend (`None` displays legend only if less than 13 entries)
default: None
legend : bool or dictionary, optional
Add a legend. If a dictionary is provided, it will be used as keyword

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W291 trailing whitespace

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @gidden, I was thinking of doing something similar already!

lgtm, one suggestion for clearer docstring...

legend : bool or dictionary, optional
Add a legend. If a dictionary is provided, it will be used as keyword
arguments in creating the legend.
default: None (displays legend only if less than 13 entries)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rephrase to 'as specified by MAX_LEGEND_LABELS'?

@gidden
Copy link
Member Author

gidden commented Jul 16, 2018 via email

region plots now use _add_legend()

add release notes

appease stickler

pep8 core.py
@gidden
Copy link
Member Author

gidden commented Jul 16, 2018

ok @danielhuppmann, this now implements a more intuitive way of setting our labels and I can now confirm that lineplot by composition works as I expect it to! Just tested with alphas.

@gidden
Copy link
Member Author

gidden commented Jul 17, 2018

@danielhuppmann did you mean to add a commit here?

@danielhuppmann
Copy link
Member

@gidden just resolved the merge conflict from accepting PR #70 so that I can merge

@danielhuppmann danielhuppmann merged commit 0877452 into IAMconsortium:master Jul 17, 2018
@gidden
Copy link
Member Author

gidden commented Jul 17, 2018 via email

@gidden gidden deleted the legends branch June 15, 2022 11:27
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