Skip to content

Re-categorize HoloViewsConverter docstring and add missing keywords in the correct special lists #1514

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.

Sign up for GitHub

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

Jump to bottom
Merged
maximlt merged 6 commits into from
Feb 28, 2025
Merged

Re-categorize HoloViewsConverter docstring and add missing keywords in the correct special lists #1514

maximlt merged 6 commits into from
Feb 28, 2025

Conversation

@Azaya89
Copy link
Collaborator

@Azaya89 Azaya89 commented Feb 24, 2025 •
edited
Loading

fixes #1513
fixes #1506

This PR:

  • re-organises the HoloViewsConverter class docstrings into more meaningful categories as follows:
  1. Data Options: Options for organizing, grouping, and transforming the dataset before visualization, including labels, sorting, and indexing.
  2. Geographic Options: Configuration options for geographic plots, including map projections, tile overlays, and geographic features like coastlines and borders.
  3. Size and Layout options: Customization options for plot sizes and overall layout, including responsive mode.
  4. Axis Options: Customization options for axis appearance and behavior, including labels, limits, tick formatting, and axis scaling.
  5. Grid and legend options: Options for setting the grid or legend of plots as well as colorbar options.
  6. Interactivity options: Bokeh specific options for adding hover tools as well as other interactive tools like tap tool and box select tool.
  7. Style Options: Visual styling options to adjust colors, fonts, plot dimensions, and other aesthetic elements of the plot.
  8. Resampling Options: Performance related options for handling large datasets, including downsampling, rasterization, and dynamic plot updates.
  9. Streaming Options: Configurations for handling live data streams, including buffering, frame management, and support for large datasets.
  • Updates the the special lists to include keywords in the docstring that are not in the special lists.

@Azaya89 Azaya89 requested a review from maximlt February 24, 2025 22:29
@Azaya89 Azaya89 self-assigned this Feb 24, 2025
@Azaya89 Azaya89 added the NF SDG 2025 NumFocus Software Development Grant 2025 label Feb 24, 2025
@Azaya89 Azaya89 force-pushed the group-docstrings1513 branch from 315fc8d to a24cd3a Compare February 24, 2025 22:36
maximlt
maximlt requested changes Feb 25, 2025
View reviewed changes
Copy link
Member

@maximlt maximlt left a comment •
edited by Azaya89
Loading

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
Copy link
Member

@maximlt maximlt Feb 25, 2025

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?

Copy link
Collaborator Author

@Azaya89 Azaya89 Feb 25, 2025

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.

@Azaya89
Copy link
Collaborator Author

Azaya89 commented Feb 25, 2025 •
edited
Loading

  • 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.

OK. Will do.

  • 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?

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 :)

@maximlt
Copy link
Member

maximlt commented Feb 25, 2025

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

Azaya89
Azaya89 commented Feb 25, 2025
View reviewed changes
@Azaya89 Azaya89 requested a review from maximlt February 25, 2025 15:57
@Azaya89 Azaya89 mentioned this pull request Feb 25, 2025
Closed
maximlt
maximlt requested changes Feb 27, 2025
View reviewed changes
@Azaya89 Azaya89 requested a review from maximlt February 27, 2025 14:35
maximlt
maximlt approved these changes Feb 28, 2025
View reviewed changes
Copy link
Member

@maximlt maximlt left a 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.

@maximlt maximlt merged commit f553dd0 into holoviz:main Feb 28, 2025
11 checks passed
@Azaya89 Azaya89 mentioned this pull request Feb 28, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maximlt maximlt maximlt approved these changes

Assignees

@Azaya89 Azaya89

Labels

NF SDG 2025 NumFocus Software Development Grant 2025

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Re-arrange the HoloViews Converter docstrings into more meaningful groups. Ensure all the generic keywords are listed in the special lists correctly

2 participants

@Azaya89 @maximlt