Skip to content

Conversation

@gforsyth
Copy link
Contributor

@gforsyth gforsyth commented Oct 21, 2025

Trying to relax the upper pin for bokeh to see what happens in CI.

We currently pin bokeh to >3.1,<=3.6 in dependencies.yaml but not in the rattler-build recipe, so we are testing a different dependency set than the one that a user will get if they install using conda.

Fixes #728

@gforsyth gforsyth requested a review from a team as a code owner October 21, 2025 13:10
@gforsyth gforsyth requested a review from msarahan October 21, 2025 13:10
@gforsyth gforsyth added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 21, 2025
@gforsyth gforsyth requested a review from a team as a code owner October 21, 2025 13:35
…trictions

"custominspect" isn't a valid type for the alias.
Also, we don't need it (says Cursor) b.c. we can use it directly as a
`CustomInspectTool` instance.

In any case, I was able to run previously failing example charts after
making this change.
@gforsyth
Copy link
Contributor Author

I fixed and updated some docstring formatting because I thought warnings were getting escalated to errors -- the actual issue was in some stricter Typescript types in newer version of Bokeh.

That was fixed by removing the CustomInspectTool alias and just using the instance directly.

I can rip out the docstring fixes if we want to keep this PR more tightly scoped.

@gforsyth gforsyth changed the title fix(deps): relax bokeh upper pin fix(deps): relax bokeh upper pin, update custom TS usage Oct 21, 2025
Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Thanks Gil! 🙏

Fixed one conflict related to file header changes from PR: #734

Though noticed there are a few more places we likely want these changes

cc @KyleFromNVIDIA (to check we are capturing these correctly)

@jakirkham
Copy link
Member

Kyle submitted PR ( #736 ) for the SPDX changes

Have tried to resolve the conflicts it introduced with this PR

Looks like that captures the header changes that were originally being made here

Though please double check and make sure this looks ok

@KyleFromNVIDIA
Copy link
Member

My PR based its headers off of git log, which in some cases revealed start years other than 2019, such as 2020 and 2023. But this PR had them all at 2019. Which one is correct?

Copy link
Member

@AjayThorve AjayThorve left a comment

Choose a reason for hiding this comment

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

CI passing for the changes made in this PR is just api level confirmation. We still need some confirmation that the dashboards work in notebooks and colab, has that been tried @gforsyth ?

@gforsyth
Copy link
Contributor Author

No, I haven't checked that -- probably better if someone familiar with the package takes a look so they can verify the outputs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[QST] Dashboard giving "Compilation failed:"

4 participants