Fix type annotations for state session callbacks #8320
Conversation
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix type annotations for session lifecycle callbacks by changing the parameter type from SessionContext to BokehSessionContext in two public methods: on_session_created and on_session_destroyed.
Key Changes
- Updated type annotation for
on_session_createdcallback parameter fromSessionContexttoBokehSessionContext - Updated type annotation for
on_session_destroyedcallback parameter fromSessionContexttoBokehSessionContext
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| self._on_session_created.append(callback) | ||
|
|
||
| def on_session_destroyed(self, callback: Callable[[SessionContext], None]) -> None: | ||
| def on_session_destroyed(self, callback: Callable[[BokehSessionContext], None]) -> None: |
There was a problem hiding this comment.
The type annotation change to BokehSessionContext is inconsistent with the class variable _on_session_destroyed at line 189, which is typed as list[Callable[[SessionContext], None]]. This creates a type incompatibility when appending callbacks at line 808. Additionally, Bokeh's Document.on_session_destroyed method (called at line 804) expects Callable[[SessionContext], None], not BokehSessionContext.
The class variable at line 189 should also be updated to use BokehSessionContext for consistency, or if SessionContext is the correct type, then this change should be reverted.
| def on_session_destroyed(self, callback: Callable[[BokehSessionContext], None]) -> None: | |
| def on_session_destroyed(self, callback: Callable[[SessionContext], None]) -> None: |
| self._onload[self.curdoc] = [(callback, threaded)] | ||
|
|
||
| def on_session_created(self, callback: Callable[[SessionContext], None]) -> None: | ||
| def on_session_created(self, callback: Callable[[BokehSessionContext], None]) -> None: |
There was a problem hiding this comment.
The type annotation change to BokehSessionContext is inconsistent with the class variable _on_session_created at line 187, which is typed as list[Callable[[SessionContext], None]]. This creates a type incompatibility when appending callbacks at line 796. Additionally, internal callbacks like _initialize_session_info (line 167 in server.py) and _register_session_destroyed (line 501) use SessionContext, not BokehSessionContext.
The class variables at lines 187-189 should also be updated to use BokehSessionContext for consistency, or if SessionContext is the correct type, then this change should be reverted.
| def on_session_created(self, callback: Callable[[BokehSessionContext], None]) -> None: | |
| def on_session_created(self, callback: Callable[[SessionContext], None]) -> None: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8320 +/- ##
===========================================
+ Coverage 70.21% 85.39% +15.17%
===========================================
Files 347 348 +1
Lines 54596 54597 +1
===========================================
+ Hits 38337 46624 +8287
+ Misses 16259 7973 -8286 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixes #8307