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.

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

Handle Bokeh 3.0 CDSView change #5643

Merged
merged 2 commits into from
Jan 10, 2022
Merged

Conversation

bryevdv
Copy link
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

Copy link
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

distributed/dashboard/utils.py Outdated Show resolved Hide resolved
@bryevdv
Copy link
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
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.

Copy link
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
@jrbourbeau
Copy link
Member

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

@bryevdv
Copy link
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants