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

fixes for bugs associating display IDs across cells #135

Merged
merged 24 commits into from
Dec 14, 2022
Merged

Conversation

shouples
Copy link
Collaborator

@shouples shouples commented Dec 13, 2022

During resample/assignment requests, results from duckdb queries would be assigned to SUBSET_TO_DISPLAY_ID. This is used when handle_format() is called for any dataframe object to determine whether it's updating an existing display handler or if it's creating a new one.

Changes:

  • explicitly disables assigning duckdb results to SUBSET_TO_DISPLAY_ID during a variable assignment request
  • checks display ID and cell ID for any SUBSET_TO_DISPLAY_ID match before rendering/formatting a dataframe output, and falls back to using the kernel sidecar supplied LAST_EXECUTED_CELL_ID if no parent cell ID is found
    • cell ID is then checked on any subsequent format() calls to ensure we aren't trying to update display handlers across different cell executions
  • adds function to register/unregister comms based on ENABLE_DATALINK/ENABLE_RENAME/ENABLE_ASSIGNMENT settings changes

@shouples shouples added this to the DEX to Kernel Sync milestone Dec 13, 2022
@shouples shouples self-assigned this Dec 13, 2022
@shouples shouples marked this pull request as draft December 13, 2022 22:40
@codecov-commenter
Copy link

codecov-commenter commented Dec 14, 2022

Codecov Report

Merging #135 (7a35183) into main (ed9e1e7) will increase coverage by 0.09%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
+ Coverage   80.03%   80.13%   +0.09%     
==========================================
  Files          48       48              
  Lines        2399     2436      +37     
==========================================
+ Hits         1920     1952      +32     
- Misses        479      484       +5     
Impacted Files Coverage Δ
src/dx/comms/assignment.py 78.04% <ø> (ø)
src/dx/settings.py 83.53% <71.42%> (-1.14%) ⬇️
src/dx/formatters/main.py 84.46% <82.35%> (-0.15%) ⬇️
src/dx/filtering.py 100.00% <100.00%> (ø)
src/dx/utils/tracking.py 95.60% <100.00%> (+0.54%) ⬆️
src/dx/utils/formatting.py 87.11% <0.00%> (+0.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed9e1e7...7a35183. Read the comment docs.

@shouples shouples marked this pull request as ready for review December 14, 2022 02:13
@shouples shouples changed the title fixes for display ID confusion fixes for bugs associating display IDs across cells Dec 14, 2022
@shouples shouples merged commit c516580 into main Dec 14, 2022
@shouples shouples deleted the djs/datalink-fixes branch December 14, 2022 03:28
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