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

Restore whether the variable is a dataframe on load #159

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

jonatanklosko
Copy link
Member

Fixes livebook-dev/livebook#2547.

We generate code conditionally depending on whether the variable is a dataframe. Currently we compute that on the fly based on the list of dataframe variables we know. However, when the runtime just started, this list is empty. This PR changes it such that we restore the value of is_data_frame and only change it when the variable is available.

"assign_to" => nil,
"lazy" => true,
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm pretty sure we don't use this anywhere.

@@ -967,11 +973,6 @@ defmodule KinoExplorer.DataTransformCell do

defp implements?(protocol, value), do: protocol.impl_for(value) != nil

defp is_data_frame?(ctx) do
df = ctx.assigns.root_fields["data_frame"]
Map.get(ctx.assigns.data_frame_variables, df)
Copy link
Member Author

Choose a reason for hiding this comment

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

Sidenote: the reason for the linked issue is that here, if we have no variable information, we would return nil, which fails the guard defp lazy(nodes, %{is_data_frame: false}), do: nodes, and consequently produces a read-lazy code, which may not be applicable.

@jonatanklosko jonatanklosko merged commit 18c1416 into main Apr 9, 2024
1 check passed
@jonatanklosko jonatanklosko deleted the jk-store-dataframe-flag branch April 9, 2024 12:03
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.

Race condition on smart cell evaluation
2 participants