8000 Safe embed [doc-build] by philippjfr · Pull Request #1040 · holoviz/panel · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@philippjfr
Copy link
Member

Reverts #1021

Just to recap some of the conversation/explanation from that PR:

Just to explain both the issues you are having with HoloViews and what this workaround is meant to achieve. Embedding in Panel (and previously the HoloMap support in HoloViews) work by collecting all the widgets in an app that have a discrete set of values and then taking their cross product to come up with the state space. The embedding code then iterates over that state space and records the events that are generated as each change is applied. This works well for simple cases but can exhibit issues with more complex cases when combined with HoloViews. Specifically HoloViews tries to compute the minimal set of events to go from one state to the next. Imagine you have states A, B and C, the embedding code will go from A->B and record the events, and then go from B->C and record the events. However, since it's computing minimal diffs the transition from C->B may not include all the events to fully restore the view to the original state A, because it has only recorded the diff to go from A->B and A and B may be more similar than C and A. This problem is particularly acute when we change column labels.

The correct solution would be to ensure that during embedding HoloViews always includes all the events necessary to fully restore the state. An alternative solution would be to find a path through the state space that applies all the events to get to that state consecutively. Both are pretty difficult problems, particularly while also still minimizing the size of the resulting data. This PR is a total hammer to solve this problem, instead of saying "let's try to compute a minimal diff between A and B" it says, let's just embed the entire plot for each state change. This will result in much bigger file sizes and trigger a full rerender of the plot but it also guarantees all the state changes are recorded in full.

The new approach in this PR provides a context manager which simply short circuits bokeh's mechanism to see if some property has changed or not, always telling it that the property has changed and an event should be generated. This means that for each state change all properties that are altered will be recorded. This is still no 100% guarantee that all changes are captured since a user might have conditional logic but it should capture the vast majority of cases in HoloViews and using changes in HoloViews we can make sure that all necessary events are captured.

@codecov
Copy link
codecov bot commented Jan 29, 2020

Codecov Report

Merging #1040 into master will increase coverage by 0.01%.
The diff coverage is 87.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1040      +/-   ##
==========================================
+ Coverage   85.76%   85.78%   +0.01%     
==========================================
  Files         105      105              
  Lines       12019    12275     +256     
==========================================
+ Hits        10308    10530     +222     
- Misses       1711     1745      +34
Impacted Files Coverage Δ
panel/pane/holoviews.py 84.63% <100%> (+1.42%) ⬆️
panel/io/notebook.py 62.71% <100%> (+0.58%) ⬆️
panel/config.py 48.27% <100%> (+1.6%) ⬆️
panel/io/embed.py 88.55% <71.42%> (+0.03%) ⬆️
panel/io/save.py 85.48% <0%> (+8.12%) ⬆️
panel/viewable.py 85.58% <0%> (+3.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2701968...7e3c169. Read the comment docs.

@github-actions
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.

73A2

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0