Skip to content

Handle Bokeh 3.0 CDSView change#5643

Merged
jrbourbeau merged 2 commits intodask:mainfrom
bryevdv:bryanv/cdsview_3_0
Jan 10, 2022
Merged

Handle Bokeh 3.0 CDSView change#5643
jrbourbeau merged 2 commits intodask:mainfrom
bryevdv:bryanv/cdsview_3_0

Conversation

@bryevdv
Copy link
Copy Markdown
Contributor

@bryevdv bryevdv commented Jan 7, 2022

Starting with Bokeh 3.0, CDSView will automatically infer the correct ColumnDatSource to apply to, and therefore no longer has a source property. See

This PR adds an version check to explicitly switch appropriate behavior.

  • Tests added / passed
  • Passes pre-commit run --all-files

NOTE: just checking CI at this point, I still need to actually manually test with 3.0.0dev

jrbourbeau
jrbourbeau reviewed Jan 7, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @bryevdv -- just as a heads up there are a few known flaky tests, so if you see seemingly unrelated test failures in CI, that is most likely the cause

@bryevdv
Copy link
Copy Markdown
Contributor Author

bryevdv commented Jan 7, 2022

@jrbourbeau AFAICT everything is working fine with both 2.4.2 and 3.0.0dev

I am seeing an error message on exit with 3.0dev

ValueError: failed to validate DataRange1d(id='10668', ...).start: expected an element of either Float, Datetime or TimeDelta, got {'$type': 'number', 'value': 'nan'}
bokeh.server.protocol_handler - ERROR - error handling message
 message: Message 'PATCH-DOC' content: {'events': [{'kind': 'ModelChanged', 'model': {'id': '10702'}, 'attr': 'end', 'new': {'$type': 'number', 'value': '-inf'}}], 'references': []}
 error: ValueError("failed to validate DataRange1d(id='10702', ...).end: expected an element of either Float, Datetime or TimeDelta, got {'$type': 'number', 'value': '-inf'}")

But that seems like something to handle in a separate issue

@bryevdv
Copy link
Copy Markdown
Contributor Author

bryevdv commented Jan 10, 2022

@jrbourbeau I did dig into the three failed tests, all "macos, not ci1" tests and all with a few failures related to reconnection and/or timeouts. I don't think they pertain to anything Bokeh-related but any guidance is welcome.

jrbourbeau
jrbourbeau approved these changes Jan 10, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @bryevdv, those tests are definitely unrelated to the changes here

@jrbourbeau jrbourbeau merged commit b9f657b into dask:main Jan 10, 2022
@jrbourbeau jrbourbeau mentioned this pull request Jan 10, 2022
Merged
@jrbourbeau
Copy link
Copy Markdown
Member

jrbourbeau commented Jan 10, 2022

FWIW I'm running the full CI suite against the development version of bokeh over in #5648 to see what happens

@bryevdv
Copy link
Copy Markdown
Contributor Author

bryevdv commented Jan 10, 2022

Thanks @jrbourbeau !

@bryevdv bryevdv deleted the bryanv/cdsview_3_0 branch January 10, 2022 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jrbourbeau jrbourbeau jrbourbeau approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bryevdv @jrbourbeau