From 670611ded1acab7c12711444c0300b6bc7267274 Mon Sep 17 00:00:00 2001 From: Nathaniel Nicandro Date: Sat, 12 May 2018 14:52:35 -0500 Subject: [PATCH] Update code comments and documentation --- jupyter-client.el | 44 +++++++++++++++------------ jupyter-kernel-manager.el | 5 ++-- jupyter-repl-client.el | 60 +++++++++++++++++++++++++------------ ob-jupyter.el | 62 ++++++++++++++++++++++++--------------- 4 files changed, 108 insertions(+), 63 deletions(-) diff --git a/jupyter-client.el b/jupyter-client.el index e766b51..10b0674 100644 --- a/jupyter-client.el +++ b/jupyter-client.el @@ -208,8 +208,8 @@ connection is terminated before initializing a new one." (otherwise (error "Wrong channel type"))) ;; So channels have access to the client's session ;; - ;; See `jupyter-start-channels' for when the :ioloop slot is - ;; set + ;; See `jupyter-start-channels' for when the :ioloop slot of + ;; a channel is set :session session :endpoint (funcall addr port)))))))) @@ -271,11 +271,11 @@ this is called." message) "Send a message on CLIENT's CHANNEL. Return a `jupyter-request' representing the sent message. CHANNEL -is one of the channel's of CLIENT. TYPE is one of the values in -`jupyter-message-types' and is the type of the MESSAGE. If FLAGS -is non-nil, it has the same meaning as FLAGS in `zmq-send'. You -can manipulate how to handle messages received in response to the -sent message, see `jupyter-add-callback' and +is one of the channel's of CLIENT. TYPE is one of the +`jupyter-message-types'. MESSAGE is the message sent on CHANNEL. + +Note that you can manipulate how to handle messages received in +response to the sent message, see `jupyter-add-callback' and `jupyter-request-inhibited-handlers'." (declare (indent 1)) (let ((ioloop (oref client ioloop))) @@ -358,12 +358,13 @@ Any other command sent to the subprocess will be ignored." (signal 'quit nil)) (otherwise (error "Unhandled command (%s)" cmd))))) -;; This may not happen if the parent emacs crashes. One solution is to send the -;; process id of the parent emacs and periodically check if the process is -;; still alive, then exit the subprocess if the parent process is dead. +;; FIXME: The subprocess may not get killed if the parent emacs crashes. One +;; solution to this is to send the process id of the parent emacs and +;; periodically check if the process is still alive, then exit the subprocess +;; if the parent process is dead. ;; -;; TODO: Fix the problem where lots of display_data messages are coming in, we -;; send a request, and wait for the request id to come back with +;; FIXME: Fix the problem where lots of display_data messages are coming in, +;; then we send a request, and wait for the request id to come back with ;; `jupyter-request-id'. `jupyter-request-id' will time out. it looks like the ;; poller is not noticing the stdin event in this case. (defun jupyter--ioloop (client) @@ -423,7 +424,9 @@ Any other command sent to the subprocess will be ignored." ;; ;; TODO: Drop messages if they are comming too frequently ;; to the point where the parent Emacs process would be - ;; spending too much time handling messages. + ;; spending too much time handling messages. Or better + ;; yet, reduce the rate at which messages are being sent + ;; to the parent process. (when (and messages (or (= idle-count 5) (> (length messages) 10))) (setq messages (nreverse messages)) @@ -527,6 +530,10 @@ by `jupyter--ioloop'." ;;; Starting the channel subprocess (defun jupyter-ioloop-wait-until (event ioloop &optional timeout) + "Wait until EVENT occurs in IOLOOP. +Currently EVENT can be :start or :quit and this function will +blocks for TIMEOUT seconds until IOLOOP starts or quits depending +on EVENT. If TIMEOUT is nil it defaults to 1 s." (or timeout (setq timeout 1)) (with-timeout (timeout nil) (while (null (process-get ioloop event)) @@ -534,6 +541,7 @@ by `jupyter--ioloop'." t)) (defun jupyter--start-ioloop (client) + "Start CLIENT's channels." (unless (oref client ioloop) (oset client ioloop (zmq-start-process @@ -608,10 +616,10 @@ See `jupyter-add-callback'." (defun jupyter--add-callback (req msg-type cb) "Helper function for `jupyter-add-callback'. -REQ is a `jupyter-request' object, MSG-TYPE should be one of the +REQ is a `jupyter-request' object, MSG-TYPE is one of the keywords corresponding to a received message type in -`jupyter-message-types', and CB will be the callback that will be -run when MSG-TYPE is received for REQ." +`jupyter-message-types', and CB is the callback that will be run +when MSG-TYPE is received for REQ." (setq msg-type (or (plist-get jupyter-message-types msg-type) ;; A msg-type of t means that FUNCTION is run for all ;; messages associated with a request. @@ -691,8 +699,8 @@ within TIMEOUT. Note that if no TIMEOUT is given, it defaults to (defun jupyter-wait-until-received (msg-type req &optional timeout) "Wait until a message of a certain type is received for a request. -MSG-TYPE and REQ has the same meaning as their corresponding -argument in `jupyter-add-callback'. If no message that matches +MSG-TYPE and REQ have the same meaning as their corresponding +arguments in `jupyter-add-callback'. If no message that matches MSG-TYPE is received for REQ within TIMEOUT seconds, return nil. Otherwise return the first message that matched MSG-TYPE. Note that if no TIMEOUT is given, it defaults to diff --git a/jupyter-kernel-manager.el b/jupyter-kernel-manager.el index b71b80d..c4f50fd 100644 --- a/jupyter-kernel-manager.el +++ b/jupyter-kernel-manager.el @@ -92,7 +92,7 @@ SLOTS are the slots used to initialize the client with.") (cl-defmethod jupyter-make-client ((manager jupyter-kernel-manager) class &rest slots) "Make a new client from CLASS connected to MANAGER's kernel. CLASS should be a subclass of `jupyter-kernel-client', a new -instance of CLASS initialized with SLOTS and configured to +instance of CLASS is initialized with SLOTS and configured to connect to MANAGER's kernel." (unless (child-of-class-p class 'jupyter-kernel-client) (signal 'wrong-type-argument (list '(subclass jupyter-kernel-client) class))) @@ -105,7 +105,8 @@ connect to MANAGER's kernel." "Cleanup resources after kernel shutdown. If MANAGER's KERNEL process terminates, i.e. when EVENT describes an event in which the KERNEL process was killed: kill the process -buffer and delete MANAGER's conn-file." +buffer and delete MANAGER's connection file from the +`jupyter-runtime-directory'." (cond ((not (process-live-p kernel)) (and (buffer-live-p (process-buffer kernel)) diff --git a/jupyter-repl-client.el b/jupyter-repl-client.el index 077d1b4..cec52c4 100644 --- a/jupyter-repl-client.el +++ b/jupyter-repl-client.el @@ -26,8 +26,28 @@ ;; A Jupyter REPL for Emacs. ;; ;; The main entry points are `run-jupyter-repl' and `connect-jupyter-repl'. -;; `run-jupyter-repl' starts a new kernel, connects a `jupyter-repl-client' to -;; it, and pops up a REPL buffer when called interactively. Whereas `connect-jupyter-repl' +;; +;; When called interactively, `run-jupyter-repl' asks for a kernel to start +;; (based on the kernels found using `jupyter-available-kernelspecs'), connects +;; a `jupyter-repl-client' to the selected kernel, and pops up a REPL buffer. +;; On the other hand, if `connect-jupyter-repl' is called interactively, it +;; will ask for the JSON file that contains the kernel's connection info. +;; +;; Additionally, `jupyter-repl-associate-buffer' associates the +;; `current-buffer' with a REPL client appropriate for the buffer's +;; `major-mode'. Associating a buffer with a REPL client enables the minor mode +;; `jupyter-repl-interaction-mode' and, if `company-mode' is installed, enables +;; auto-completion using the associated REPL client. +;; +;; `jupyter-repl-interaction-mode' adds the following keybindings for +;; interacing a REPL client: +;; +;; C-c C-c `jupyter-repl-eval-line-or-region' +;; C-c C-l `jupyter-repl-eval-file' +;; C-c C-f `jupyter-repl-inspect-at-point' +;; C-c C-r `jupyter-repl-restart-kernel' +;; C-c C-i `jupyter-repl-interrupt-kernel' +;; C-c C-z `jupyter-repl-pop-to-buffer' ;;; Code: @@ -451,7 +471,8 @@ image." (jupyter-repl-insert tex) (setq end (point)) (org-format-latex - "jupyter-repl" beg end "jupyter-repl" + ;; FIXME: Possibly make a resource directory for the REPL + "ltx" beg end org-babel-jupyter-resource-directory 'overlays "Creating LaTeX image...%s" 'forbuffer ;; Use the default method for creating image files @@ -1035,6 +1056,9 @@ buffer to display TEXT." (let ((pos (point))) (jupyter-repl-insert-ansi-coded-text (mapconcat #'identity traceback "\n")) + ;; TODO: Better way of accessing client kernel's properties + ;; + ;; (jupyter-client-property client :kernel-language) (when (eq jupyter-repl-lang-mode 'python-mode) ;; Fix spacing between error name and Traceback (save-excursion @@ -1046,6 +1070,7 @@ buffer to display TEXT." (jupyter-repl-insert (make-string (if (> len 4) len 4) ? ))))))) (jupyter-repl-newline))) + ;; TODO: Probably this shouldn't be here. (jupyter-repl-display-other-output client "stderr" (format "(other client) %s: %s" ename evalue)))) @@ -1073,9 +1098,11 @@ REPL buffer." thereis (eq (ring-ref jupyter-repl-history -1) 'jupyter-repl-history) do (ring-insert jupyter-repl-history (ring-remove jupyter-repl-history -1))) + ;; When the next history element is the sentinel, handle some edge cases (cond ((equal (jupyter-repl-cell-code) (ring-ref jupyter-repl-history 0)) + ;; If the cell code is the last history item, erase it (jupyter-repl-replace-cell-code "") (setq no-replace t)) ((equal (jupyter-repl-cell-code) "") @@ -1272,8 +1299,9 @@ kernel that the REPL buffer is connected to." (defun jupyter-repl-preserve-window-margins (&optional window) "Ensure that the margins of a REPL window are present. This function is added as a hook to `pre-redisplay-functions' to -ensure that a REPL windows margins are present. If WINDOW is -showing a REPL buffer and the margins are not set to +ensure that a REPL windows margins are present. + +If WINDOW is showing a REPL buffer and the margins are not set to `jupyter-repl-prompt-margin-width', set them to the proper value." (when (and (eq major-mode 'jupyter-repl-mode) @@ -1287,18 +1315,13 @@ value." (defun jupyter-repl-code-context-at-point (type) "Return a cons cell, (CODE . POS), for the context around `point'. -Returns the required context depending on TYPE which can be -either `inspect' or `complete'. If TYPE is `inspect' return an -appropriate context for an inspect request. If TYPE is `complete' -return an appropriate context for a completion request. PREFIX -should be the prefix of the completion when TYPE is `complete'. -PREFIX is unused when TYPE is `inspect'. - -The context also depends on the `major-mode' of the -`current-buffer'. If the `current-buffer' is a -`jupyter-repl-mode' buffer, CODE is the contents of the entire -code cell. Otherwise its either the line up to `point' if TYPE is -`complete' or the entire line if TYPE is `inspect'." +CODE is the required context for TYPE (either `inspect' or +`complete') and POS is the relative position of `point' within +CODE. The context also depends on the `major-mode' of the +`current-buffer'. If the `major-mode' is `jupyter-repl-mode', +CODE is the contents of the entire code cell. Otherwise its +either the line up to `point' if TYPE is `complete' or the entire +line if TYPE is `inspect'." (unless (memq type '(complete inspect)) (error "Type not `complete' or `inspect' (%s)" type)) (let (code pos) @@ -1597,7 +1620,6 @@ A kernel can be interrupted if it was started using a (oref jupyter-repl-current-client manager)))) ;; TODO: Make timeouts configurable -;; TODO: Handle all consequences of a shutdown (defun jupyter-repl-restart-kernel (shutdown) "Restart the kernel. With a prefix argument, SHUTDOWN the kernel completely instead." @@ -2001,7 +2023,7 @@ Otherwise, in a non-interactive call, return the (setq kernel-name (caar (jupyter-find-kernelspecs kernel-name)))) (unless kernel-name (error "No kernel found for prefix (%s)" kernel-name)) - ;; The manager is set as the client's parent-instance in + ;; The manager is set as the client's manager slot in ;; `jupyter-start-new-kernel' (cl-destructuring-bind (_manager client info) (jupyter-start-new-kernel kernel-name 'jupyter-repl-client) diff --git a/ob-jupyter.el b/ob-jupyter.el index d171f9b..578a795 100644 --- a/ob-jupyter.el +++ b/ob-jupyter.el @@ -201,7 +201,7 @@ is used as the extension." (concat (file-name-as-directory dir) (sha1 data) ext))) (defun org-babel-jupyter--image-result (data file &optional overwrite base64-encoded) - "Possibly write DATA to FILE. + "Possibly write image DATA to FILE. If OVERWRITE is non-nil, overwrite FILE if it already exists. Otherwise if FILE already exists, DATA is not written to FILE. @@ -224,7 +224,14 @@ Return the cons cell (\"file\" . FILE), see (defun org-babel-jupyter-prepare-result (data metadata params) "Return the rendered DATA. -DATA is a plist, (:mimetype1 value1 ...), which is used to render +DATA is converted into a representation suitable for display in +an `org-mode' buffer depending on + +DATA is a plist, (:mimetype1 value1 ...), containing the +different representations of a result returned by a kernel. +Preparing a result + +which is used to render a result which can be passed to `org-babel-insert-result'. METADATA is the metadata plist used to render DATA with, as @@ -233,17 +240,18 @@ information such as the size of an image to be rendered. The metadata plist is currently unused. PARAMS is the source block parameter list as passed to -`org-babel-execute:jupyter'. Currently this is only used to -extract the file name of an image file when DATA can be rendered -as an image type (either `:image/png' or `:image/svg+xml') when a -file name is passed to the code block. If no file name is given -one is generated based on DATA and the mimetype, see -`org-babel-jupyter-file-name'. +`org-babel-execute:jupyter'. Currently this is used to extract +the file name of an image file when DATA can be rendered as an +image. If no file name is given, one is generated based on the +image data and mimetype, see `org-babel-jupyter-file-name'. +PARAMS is also used to intelligently choose the rendering +parameter used for result insertion. -This function returns a cons cell (RESULT-PARAM . RESULT) where -RESULT-PARAM is either a result parameter, i.e. one of the result -paramters of `org-babel-insert-result', or a key value pair which -should be appended to the PARAMS list when to render RESULT. +This function returns a cons cell (RENDER-PARAM . RESULT) where +RENDER-PARAM is either a result parameter, i.e. one of the result +parameters of `org-babel-insert-result', or a key value pair +which should be appended to the PARAMS list when rendering +RESULT. For example, if DATA only contains the mimetype `:text/markdown', the RESULT-PARAM will be @@ -386,20 +394,26 @@ language." (defun org-babel-jupyter-insert-results (results params kernel-lang) "Insert RESULTS at the current source block location. -RESULTS is either a single pair or a list of pairs, each pair -having the form +RESULTS is either a single cons cell or a list of such cells, +each cell having the form (RENDER-PARAM . RESULT) -i.e. the pairs returned by `org-babel-jupyter-prepare-result'. -PARAMS are the parameters passed to `org-babel-execute:jupyter'. -KERNEL-LANG is the language of the kernel that produced RESULTS. +They should have been collected by previous calls to +`org-babel-jupyter-prepare-result'. PARAMS are the parameters +passed to `org-babel-execute:jupyter'. KERNEL-LANG is the +language of the kernel that produced RESULTS. -Note that for a list of results, the result which will appear -will be the last one in the list unless the source block has an -\"append\" or \"prepend\" parameter or some other way that -prevents `org-babel-insert-result' from clearing a result when -inserting a new one." +Note that if RESULTS is a list, the last result in the list will +be the one that eventually is shown in the org document. This is +due to how `org-babel-insert-result' works. This behavior can be +modified if the source block has an \"append\" or \"prepend\" +parameter; in this case results will either be appended or +prepended. + +The current implementation of `org-babel-execute:jupyter' will +automatically add this parameter internally so under normal use +it does not need to be added by the user." ;; Unless this is a list of results (unless (car-safe (car results)) (setq results (list results))) @@ -418,8 +432,8 @@ inserting a new one." (defun org-babel-execute:jupyter (body params) "Execute BODY according to PARAMS. -BODY is the code to execute for the current Jupyter `:session' of -PARAMS." +BODY is the code to execute for the current Jupyter `:session' in +the PARAMS alist." (let* ((repl-buffer (org-babel-jupyter-initiate-session (alist-get :session params) params)) (client (with-current-buffer repl-buffer