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

Cleaned up and improved legend handling for all backends #434

Merged
merged 9 commits into from
Feb 3, 2016

Conversation

philippjfr
Copy link
Member

As the title suggests this is a refactor of the legend code so that a) it behaves more similarly across backends and b) to clean up the code a little bit.

Concretely this PR addresses the following issues:

  • Legends not updating per frame in mpl
  • Legend zorder being too low in mpl
  • Allow toggling legends consistently at the Element and Overlay level, providing finer control. Previously some Element types could be toggled others could not.

@philippjfr philippjfr added this to the v1.4.2 milestone Feb 2, 2016
@philippjfr philippjfr self-assigned this Feb 2, 2016
@philippjfr
Copy link
Member Author

This PR is now ready. While not perfect, legend behavior should now be much more consistent and I've tidied up the matplotlib implementation a little bit.

@@ -264,6 +264,9 @@ class SpikesPlot(PathPlot):
position = param.Number(default=0., doc="""
The position of the lower end of each spike.""")

show_legend = param.Boolean(default=True, doc="""
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a parameter available for the matplotlib backend IIRC. In which case, it is good to see this support added to the Bokeh backend.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's always been there I'm just being explicit about which plots have it enabled and disabled by default.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok good.

@jlstevens
Copy link
Contributor

Looks good and I am happy to merge.

This is the first PR to use the new documentation building system. I'm really happy with how easy it was to update the reference data for this PR. Note that when I click merge, this should trigger a merge hook but this will be the first time it is tested - we may have a failure on master if something goes wrong with the test data update.

jlstevens added a commit that referenced this pull request Feb 3, 2016
Cleaned up and improved legend handling for all backends
@jlstevens jlstevens merged commit 08ac4b8 into master Feb 3, 2016
@jlstevens jlstevens deleted the legend_fixes branch February 4, 2016 16:07
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants