ipywidgets: thinking through sending buffers from client to kernel

Author: williamstein

src/packages/frontend/jupyter/output-messages/ipywidget.tsx

@@ -29,7 +29,7 @@ interface WidgetProps {
 }
 
 export function IpyWidget({ value, actions }: WidgetProps) {
-  console.log("IpyWidget", { value: value.toJS(), actions });
+  // console.log("IpyWidget", { value: value.toJS(), actions });
   const [unknown, setUnknown] = useState<boolean>(false);
   const divRef = useRef<any>(null);
 

src/packages/frontend/jupyter/widgets/manager2.ts

@@ -139,26 +139,21 @@ export class WidgetManager {
     model.set_state(state);
   };
 
+  // ipywidgets_state_ValueChange is called when a value entry of the ipywidgets_state
+  // table is changed, e.g., when the backend decides a model should change or another client
+  // changes something, or even this client changes something.  When another client is
+  // responsible for the change, we make the change to the ipywidgets model here.
   private ipywidgets_state_ValueChange = async (model_id: string) => {
     // log("handleValueChange: ", model_id);
     const changed = this.ipywidgets_state.get_model_value(model_id);
     log("handleValueChange: changed=", model_id, changed);
-    for (const k in changed) {
-      if (typeof changed[k] == "string" && changed[k].startsWith("IPY_MODEL")) {
-        delete changed[k];
-      } else if (is_array(changed[k]) && typeof changed[k][0] == "string") {
-        if (changed[k][0]?.startsWith("IPY_MODEL")) {
-          delete changed[k];
-        }
-      }
-    }
     if (
       this.last_changed[model_id] != null &&
       changed.last_changed != null &&
       changed.last_changed <= this.last_changed[model_id].last_changed
     ) {
       log(
-        "handleValueChange: skipping due to last change time",
+        "handleValueChange: skipping due to last_changed sequence number -- i.e., change caused by this client",
         this.last_changed[model_id],
         changed.last_changed,
       );
@@ -273,10 +268,15 @@ export class WidgetManager {
     const model = await this.manager.get_model(model_id);
     model.on("change", this.handleModelChange);
     this.watching.add(model_id);
+    this.last_changed[model_id] = { last_changed: 0 };
   };
 
+  // handleModelChange is called when an ipywidgets model changes.
+  // This function serializes the change and saves it to
+  // ipywidgets_state, so that it is gets sync'd to the backend
+  // and any other clients.
   private handleModelChange = async (model): Promise<void> => {
-    // log("handleModelChange", model);
+    log("handleModelChange", model);
     const { model_id } = model;
     await model.state_change;
     if (this.state_lock.has(model_id)) {
@@ -292,21 +292,18 @@ export class WidgetManager {
     }
     // increment sequence number.
     changed.last_changed =
-      Math.max(
-        last_changed ?? 0,
-        this.last_changed[model_id]?.last_changed ?? 0,
-      ) + 1;
+      Math.max(last_changed ?? 0, this.last_changed[model_id].last_changed) + 1;
     this.last_changed[model_id] = changed;
-    // log("handleModelChange", changed);
-    this.ipywidgets_state.set_model_value(model_id, changed, true);
+    log("handleModelChange", changed);
+    this.ipywidgets_state.set_model_value(model_id, changed, false);
     this.ipywidgets_state.save();
   };
 
   private deserializeState = async (
     model: base.DOMWidgetModel,
     serialized_state: ModelState,
   ): Promise<ModelState> => {
-    log("deserializeState", { model, serialized_state });
+    // log("deserializeState", { model, serialized_state });
     // NOTE: this is a reimplementation of soemething in
     //     ipywidgets/packages/base/src/widget.ts
     // but we untagle unpacking and deserializing, which is
@@ -386,7 +383,7 @@ export class WidgetManager {
   };
 
   private dereferenceModelLinks = async (state): Promise<boolean> => {
-    log("dereferenceModelLinks", "BEFORE", state);
+    // log("dereferenceModelLinks", "BEFORE", state);
     for (const key in state) {
       const val = state[key];
       if (typeof val === "string") {
@@ -430,7 +427,7 @@ export class WidgetManager {
         }
       }
     }
-    log("dereferenceModelLinks", "AFTER (success)", state);
+    // log("dereferenceModelLinks", "AFTER (success)", state);
     return true;
   };
 }
@@ -442,7 +439,7 @@ class Environment implements WidgetEnvironment {
   }
 
   async getModelState(model_id) {
-    log("getModelState", model_id);
+    // log("getModelState", model_id);
     if (this.manager.ipywidgets_state.get_state() != "ready") {
       await once(this.manager.ipywidgets_state, "ready");
     }

src/packages/jupyter/redux/project-actions.ts

@@ -1312,6 +1312,10 @@ export class JupyterActions extends JupyterActions0 {
     });
   }
 
+  // handle_ipywidgets_state_change is called when the project ipywidgets_state
+  // object changes, e.g., in response to a user moving a slider in the browser.
+  // It crafts a comm message that is sent to the running Jupyter kernel telling
+  // it about this change by calling send_comm_message_to_kernel.
   private handle_ipywidgets_state_change(keys): void {
     if (this.is_closed()) {
       return;
@@ -1338,7 +1342,20 @@ export class JupyterActions extends JupyterActions0 {
           data,
         );
       } else if (type === "buffers") {
-        // nothing to do on the backend (?)
+        // TODO: we will implement this soon.  A good example where this is required
+        // is by the file upload widget:
+        //    https://ipywidgets.readthedocs.io/en/latest/examples/Widget%20List.html#file-upload
+        // which creates a buffer from the content of the file, then sends it to the backend,
+        // which sees a change and has to write that buffer to the kernel (here) so that
+        // the running python process can actually do something with the file contents (e.g.,
+        // process data, save file to disk, etc).
+        // We need to be careful though to not send buffers to the kernel that the kernel sent us,
+        // since that would be a waste.
+        // TODO
+        // I think the format for the send_comm_mmessage_to_kernel call here is easy to deduce
+        // from what process_comm_message_from_kernel unpacks.
+        //   ***
+        // TODO
       } else if (type === "state") {
         // TODO: currently ignoring this, since it seems chatty and pointless,
         // and could lead to race conditions probably with multiple users, etc.

src/packages/sync/editor/generic/ipywidgets-state.ts

@@ -281,7 +281,7 @@ export class IpywidgetsState extends EventEmitter {
 
   private set_model_buffers = (
     model_id: string,
-    buffer_paths: string[],
+    buffer_paths: string[][],
     buffers: Buffer[],
     fire_change_event: boolean = true,
   ): void => {
@@ -294,7 +294,7 @@ export class IpywidgetsState extends EventEmitter {
     }
     for (let i = 0; i < buffer_paths.length; i++) {
       const key = JSON.stringify(buffer_paths[i]);
-      // we set to the sha1 of the buffer not to make getting
+      // we set to the sha1 of the buffer not just to make getting
       // the buffer easy, but to make it easy to KNOW if we
       // even need to get the buffer.
       const hash = sha1(buffers[i]);
@@ -388,14 +388,15 @@ export class IpywidgetsState extends EventEmitter {
     this.set_state("closed");
   };
 
-  private dbg(_f): Function {
+  private dbg = (_f): Function => {
     if (this.client.is_project()) {
       return this.client.dbg(`IpywidgetsState.${_f}`);
     } else {
       return (..._) => {};
     }
-  }
-  public async clear(): Promise<void> {
+  };
+
+  clear = async (): Promise<void> => {
     // This is used when we restart the kernel -- we reset
     // things so no information about any models is known
     // and delete all Buffers.
@@ -417,13 +418,13 @@ export class IpywidgetsState extends EventEmitter {
       this.table.set({ string_id, type, model_id, data: null }, "none", false);
     }
     await this.table.save();
-  }
+  };
 
   // Clean up all data in the table about models that are not
   // referenced (directly or indirectly) in any cell in the notebook.
   // There is also a comm:close event/message somewhere, which
   // could also be useful....?
-  public async deleteUnused(): Promise<void> {
+  deleteUnused = async (): Promise<void> => {
     this.assert_state("ready");
     const dbg = this.dbg("deleteUnused");
     dbg();
@@ -442,13 +443,13 @@ export class IpywidgetsState extends EventEmitter {
       }
     });
     await this.table.save();
-  }
+  };
 
   // For each model in init, we add in all the ids of models
   // that it explicitly references, e.g., by IPY_MODEL_[model_id] fields
   // and by output messages and other things we learn about (e.g., k3d
   // has its own custom references).
-  public getReferencedModelIds(init: string | Set<string>): Set<string> {
+  getReferencedModelIds = (init: string | Set<string>): Set<string> => {
     const modelIds =
       typeof init == "string" ? new Set([init]) : new Set<string>(init);
     let before = 0;
@@ -470,14 +471,14 @@ export class IpywidgetsState extends EventEmitter {
     this.includeThirdPartyReferences(modelIds);
 
     return modelIds;
-  }
+  };
 
   // We find the ids of all models that are explicitly referenced
   // in the current version of the Jupyter notebook by iterating through
   // the output of all cells, then expanding the result to everything
   // that these models reference.  This is used as a foundation for
   // garbage collection.
-  private getActiveModelIds(): Set<string> {
+  private getActiveModelIds = (): Set<string> => {
     const modelIds: Set<string> = new Set();
     this.syncdoc.get({ type: "cell" }).forEach((cell) => {
       const output = cell.get("output");
@@ -497,9 +498,9 @@ export class IpywidgetsState extends EventEmitter {
       }
     });
     return this.getReferencedModelIds(modelIds);
-  }
+  };
 
-  private includeThirdPartyReferences(modelIds: Set<string>) {
+  private includeThirdPartyReferences = (modelIds: Set<string>) => {
     /*
     Motivation (RANT):
     It seems to me that third party widgets can just invent their own
@@ -545,33 +546,33 @@ export class IpywidgetsState extends EventEmitter {
         modelIds.add(model_id);
       }
     });
-  }
+  };
 
   // The finite state machine state, e.g., 'init' --> 'ready' --> 'close'
-  private set_state(state: State): void {
+  private set_state = (state: State): void => {
     this.state = state;
     this.emit(state);
-  }
+  };
 
-  public get_state(): State {
+  get_state = (): State => {
     return this.state;
-  }
+  };
 
-  private assert_state(state: string): void {
+  private assert_state = (state: string): void => {
     if (this.state != state) {
       throw Error(`state must be "${state}" but it is "${this.state}"`);
     }
-  }
+  };
 
   /*
   process_comm_message_from_kernel gets called whenever the
   kernel emits a comm message related to widgets.  This updates
   the state of the table, which results in frontends creating widgets
   or updating state of widgets.
   */
-  public async process_comm_message_from_kernel(
+  process_comm_message_from_kernel = async (
     msg: CommMessage,
-  ): Promise<void> {
+  ): Promise<void> => {
     const dbg = this.dbg("process_comm_message_from_kernel");
     // WARNING: serializing any msg could cause huge server load, e.g., it could contain
     // a 20MB buffer in it.
@@ -697,7 +698,7 @@ export class IpywidgetsState extends EventEmitter {
     }
 
     await this.save();
-  }
+  };
 
   /*
   process_comm_message_from_browser gets called whenever a
@@ -707,20 +708,20 @@ export class IpywidgetsState extends EventEmitter {
   kernel changing the value of variables (and possibly
   updating other widgets).
   */
-  public async process_comm_message_from_browser(
+  process_comm_message_from_browser = async (
     msg: CommMessage,
-  ): Promise<void> {
+  ): Promise<void> => {
     const dbg = this.dbg("process_comm_message_from_browser");
     dbg(msg);
     this.assert_state("ready");
     // TODO: not implemented!
-  }
+  };
 
   // The mesg here is exactly what came over the IOPUB channel
   // from the kernel.
 
   // TODO: deal with buffers
-  public capture_output_message(mesg: any): boolean {
+  capture_output_message = (mesg: any): boolean => {
     const msg_id = mesg.parent_header.msg_id;
     if (this.capture_output[msg_id] == null) {
       return false;
@@ -759,13 +760,13 @@ export class IpywidgetsState extends EventEmitter {
     outputs.push(mesg.content);
     this.set_model_value(model_id, { outputs });
     return true;
-  }
+  };
 
-  private async sendCustomMessage(
+  private sendCustomMessage = async (
     model_id: string,
     message: object,
     fire_change_event: boolean = true,
-  ): Promise<void> {
+  ): Promise<void> => {
     /*
     Send a custom message.
 
@@ -782,11 +783,11 @@ export class IpywidgetsState extends EventEmitter {
     // Actually, delete is not implemented for synctable, so for
     // now we just set it to an empty message.
     this.set(model_id, "message", {}, fire_change_event);
-  }
+  };
 
-  public get_message(model_id: string) {
+  get_message = (model_id: string) => {
     return this.get(model_id, "message")?.toJS();
-  }
+  };
 }
 
 // Get model id's that appear either as serialized references