Fix dynamic reports with bokeh 3.4.0 problems

[handwerkerd]

Closes #1063

I've now had 3 people ask about crashes related to this issue I'd like to figure out a nice solution to this problem.

Changes proposed in this pull request:

• Listed version numbers for bokeh in pyproject.toml
• Changed a few references from circle to scatter in dynamic_figures.py
• Since the tapping default was changed from replace to xor in bokeh v3.4.0 it was possible to select more than one component at a time and that broke other functionality. This directly defines models.TapTool(mode="replace") instead of just listing tap with the default setting.

This no longer crashes, and the static report looks fine, but there is one annoying bug relating to the dynamic interactions. I'm not sure if this is due to the above changes or due to other changes in bokeh 3.4.0. The generated report looks fine, but if you click on one component and then another, then, instead of presenting the most recent one, both are highlighted and you have to unclick each component before highlighting a new one. I could use help from someone who has played with bokeh a bit more than I have (@eurunuela @javiergcas).

[handwerkerd]

This is an example of the new problem. We should either have one highlighted component or all highlighted. I sequentially clicked on 3 components and they all remained highlighted.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.69%. Comparing base (f084dc4) to head (75a7483).
Report is 35 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1068   +/-   ##
=======================================
  Coverage   89.68%   89.69%           
=======================================
  Files          26       26           
  Lines        3482     3483    +1     
  Branches      615      615           
=======================================
+ Hits         3123     3124    +1     
  Misses        211      211           
  Partials      148      148           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[handwerkerd]

I just ran this PR with bokeh=3.4.0 and bokeh=3.3.4. The above problem does NOT happen with v3.3.4, which means this the unaddressed issue is probably due to some other change within v3.4.0 and not the code changes in this PR.

[eurunuela]

I will have a look at it.

[eurunuela]

It is not as straightforward as I had hoped. I would suggest we limit bokeh to <=3.34 if the latest version isn't necessary for us.

[handwerkerd]

I decided to ask a question on their support forum. https://discourse.bokeh.org/t/dynamic-figure-problem-with-bokeh-v-3-4-0-udgrade/11416

If we don't hear about a solution in the next few days, I'll limit this PR to 3.3.4. Assuming no one else sees issues, my changes from circle to scatter are probably good to merge since they should be backwards compatible (I haven't tested v1.0.0, but scatter was added to that version) and will be issues in the future.

[handwerkerd]

I decided to look more carefully through all the PRs merged for v3.4.0. The one that stands out to me is Add support for xor selection and inversion by mattpap · Pull Request #13545 · bokeh/bokeh · GitHub It changed the default response for taps and the new behavior I’m seeing is vaguely similar to the new default. That is I now need to tap an element a second time to deselect it before clicking on a new element.

On the bokeh discussion board, I asked whether there a way to explicitly apply the previous default so that I can confirm whether or not this is the cause of our issue. They suggested I post that Q on another board, but I also wanted to share here and see if this might be something @eurunuela can already figure out how to do.

[handwerkerd]

I think I've solved this problem. The issue was that bokeh's model.TapTool default setting for mode changed from replace to xor. In practice, this meant the variable selected_idx previously was always a single number, but, in bokeh v3.4.0 was a string of numbers.

When defining toolbars, we just included the word tap instead of explicitly defining a model.TapTool. By adding models.TapTool(mode="replace") to a few places in the code, I think it now works. There was a second weird thing where the 'hover' icon was now appearing twice to the left of the scatter plot. I think that was because it was being differently defined in different functions. (if people have opinions on the order of those icons, we can rearrange)

[handwerkerd]

@tsalo and @eurunuela This is now ready for review. One think I want to check with you is that I've now added version numbers for bokeh to pyproject.toml. Given it has version numbers, will the dependabot automatically open new PRs when future bokeh versions are released or do we need to change something else as well?

[tsalo]

Setting the minimum and maximum versions will trigger dependabot when new bokeh releases are made, so we should have forewarning of future issues like this one w.r.t. bokeh.

The donut plot in the CI looks good to me.

[handwerkerd]

@eurunuela This is my first time playing around with the bokeh parts of this code. Could you take a look to make sure this all seems good to you?

[eurunuela]

LGTM! Thank you @handwerkerd! I'm glad you found a workaround. It didn't seem very obvious when I checked, but I haven't used bokeh in a while so maybe my bokeh skills are no longer useful.