-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
Re-categorize HoloViewsConverter docstring and add missing keywords in the correct special lists #1514
Conversation
315fc8d
to
a24cd3a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start! I've left some questions as comments. A few more actions:
- Could you write in this PR a little description about the meaning of each category? I.e. what are data options? What are style options? What are axis options? We'll have to add these descriptions in the reference API eventually.
- The axis options group has ~50 keywords, that's a whole lot given the plan to have a reference page that includes them all with examples. Could you think about how to break that section down in a couple of categories?
hvplot/converter.py
Outdated
to a fixed size, ignoring any responsive option. | ||
title (default=''): str | ||
Title for the plot | ||
tools (default=[]): list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't tools be together with the hover options?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I wasn't quite sure where to fit all the hover_
related options so I left them in the generic. Maybe I can just move tools
back there.
OK. Will do.
Do you mean adding a new special list? Wouldn't that involve also using the newly created list somewhere in the code as well? Also, I know it's a long list but they kinda fit IMO :) |
Yes to both questions. You can check how plotting libraries usually distinguish between their subparts: https://plotly.com/javascript/plotly-fundamentals/, https://www.chartjs.org/docs/latest/configuration/, https://www.highcharts.com/docs/chart-concepts/understanding-highcharts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Azaya89 ! Nice improvement. As I told you, these groups are not set in stone and I expect you to need to adapt them a little when you work on documenting the options more thoroughly, same for their docstrings.
fixes #1513
fixes #1506
This PR:
HoloViewsConverter
class docstrings into more meaningful categories as follows: