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.

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

Refactor to use holoviews powered bar charts #494

Merged
merged 72 commits into from
Aug 4, 2023
Merged

Refactor to use holoviews powered bar charts #494

merged 72 commits into from
Aug 4, 2023

Conversation

AjayThorve
Copy link
Member

@AjayThorve AjayThorve commented Jun 20, 2023 •
edited
Loading

This PR refactors the bokeh charts implementation, and uses holoviews to generate the charts, while also using box-selects instead of range_sliders. This was done for 2 reasons, stability of holoviews api, which also is much more efficient, and to use streams + custom callbacks to link all charts with bar_charts.

Some of the other notable changes:

  • Using holoviews to generate bokeh plots, allows us to str columns directly for bar charts
  • Persistent non-selection view for bar_charts
  • Got rid of redundant bokeh line chart, and now a single implementation exists under datashader/line, callable cuxfilter.charts.line
  • removed preview as pyppeteer was unstable
  • Refactor to remove redundant functions and classes

@AjayThorve
remove references to reset_view and calc_datatiles
a5df88f
@AjayThorve
fix callback functions to only perform query in Dashboard class once …
6eaa8d1 
…per interaction
@AjayThorve
format as per flake8 guidelines
ee2e416
@AjayThorve
fix incorrect arguments warning for UNION
35f1887
@AjayThorve
remove references to reset_view and calc_datatiles
9fd7035
@AjayThorve
fix callback functions to only perform query in Dashboard class once …
553f247 
…per interaction
@AjayThorve
format as per flake8 guidelines
1bdf446
@AjayThorve
fix incorrect arguments warning for UNION
5b253c6
@AjayThorve
Merge branch 'improvement/remove-datatiles-support' of github.com:Aja…
d0837e3 
…yThorve/cuxfilter into improvement/remove-datatiles-support
@AjayThorve
fix imports
97a5a6d
@AjayThorve
rm datatile tests since that class is no longer supported
679b13b
@AjayThorve
remove datatile refs
83596e5
@AjayThorve
remove unused args
9c37a45
@AjayThorve
add newline
344f825
@AjayThorve
fix tests, after removing datatiles
66baaae
@AjayThorve
update readme to remove datatiles info
49d3b2a
@AjayThorve
update holoviews and panel versions, remove pydeck
2622e7d
@AjayThorve
remove default parent class variables
b506737
@AjayThorve
refactor to comply with bokeh 3.1.1
403e3a5
@AjayThorve
flake8
d7867b4
@AjayThorve
register tool
da40298
@AjayThorve
fix sizing_mode
47fbd22
@AjayThorve
added holoviews powered bar charts
67c73ce
@exactlyallan exactlyallan mentioned this pull request Jun 20, 2023
Closed
exactlyallan
exactlyallan approved these changes Aug 3, 2023
View reviewed changes
Copy link
Member

@exactlyallan exactlyallan left a comment

Choose a reason for hiding this comment

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

Lot of changes! Whats the quick rational for removing the bokeh line chart?

@AjayThorve AjayThorve removed the doc Documentation label Aug 4, 2023
@github-actions github-actions bot added the doc Documentation label Aug 4, 2023
@AjayThorve AjayThorve removed the doc Documentation label Aug 4, 2023
@AjayThorve
Copy link
Member Author

AjayThorve commented Aug 4, 2023 •
edited
Loading

Lot of changes! Whats the quick rational for removing the bokeh line chart?

Updated the description,

Adding to it: The current implementation had two line charts, one under bokeh, and another under datashader. Removed one to avoid confusion, and bokeh line's implementation that cuxfilter had wasn't really accomplishing anything that bar charts weren't.

@github-actions github-actions bot added the doc Documentation label Aug 4, 2023
@AjayThorve AjayThorve removed the doc Documentation label Aug 4, 2023
@github-actions github-actions bot added the doc Documentation label Aug 4, 2023
@AjayThorve AjayThorve removed the doc Documentation label Aug 4, 2023
@AjayThorve
Copy link
Member Author

/merge

@github-actions github-actions bot added the doc Documentation label Aug 4, 2023
@AjayThorve AjayThorve removed the doc Documentation label Aug 4, 2023
@AjayThorve AjayThorve mentioned this pull request Aug 4, 2023
Merged
@rapids-bot rapids-bot bot merged commit 8827cbb into rapidsai:branch-23.08 Aug 4, 2023
24 checks passed
This was referenced Aug 4, 2023
Closed
Closed
raydouglass pushed a commit that referenced this pull request Aug 4, 2023
@AjayThorve
Doc updates (#516)
ef65166 
Follow up to #494, updates API docs and screenshots.

cc @exactlyallan

Authors:
   - Ajay Thorve (https://github.com/AjayThorve)

Approvers:
   - Allan (https://github.com/exactlyallan)
@jameslamb jameslamb mentioned this pull request Sep 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raydouglass raydouglass raydouglass approved these changes

@exactlyallan exactlyallan exactlyallan approved these changes

Assignees
No one assigned
Labels
conda improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AjayThorve @raydouglass @exactlyallan