millejoh / emacs-ipython-notebook

Jupyter notebook client in Emacs
http://millejoh.github.io/emacs-ipython-notebook/
GNU General Public License v3.0
1.47k stars 122 forks source link

Make `ob-ein` to keep keywords in the #+RESULTS block #703

Closed yatsky closed 4 years ago

yatsky commented 4 years ago

Hi,

I was trying to stop ob-ein from removing keywords like #+NAME and #+CAPTION from my results block after executing a source block. For example, this is the original results block.

#+name: bottom-up dynamic programming approach-results
#+caption: bottom-up dynamic programming approach-results
#+RESULTS: bottom-up dynamic programming approach
#+begin_src none
8
#+end_src

After executing the src block again, I'd get this result:

#+RESULTS: bottom-up dynamic programming approach
#+begin_src none
8
#+end_src

#+name: bottom-up dynamic programming approach-results
#+caption: bottom-up dynamic programming approach-results

I basically made the following changes in ob-ein.el to achieve what I wanted (I'm using Emacs 26.3).

(defun ob-ein--execute-body (body params)
;; some code...
;; original code is
;; (org-babel-remove-result)
      (condition-case nil
          (org-babel-remove-result nil t)  ;; line 238
        (error (org-babel-remove-result)))
      *ob-ein-sentinel*)))

I also tried to create a pull request but it just couldn't pass the tests with Emacs 25.1 and I got the following error during testing:

! (/home/runner/local/cask/bin/cask eval "(let ((byte-compile-error-on-warn t)) (cask-cli/build))" 2>&1 | egrep -a "(Warning|Error):") ; (ret=$? ; /home/runner/local/cask/bin/cask clean-elc && exit $ret)
lisp/ob-ein.el:238:12:Error: org-babel-remove-result called with 2 arguments, but accepts only 0-1
make: *** [test-compile] Error 1
Makefile:59: recipe for target 'test-compile' failed
##[error]Process completed with exit code 2.

This is quite strange to me as I'm already using (condition-case) with (error) but the program seems to be ignoring what I wrote there (also tried (wrong-number-of-arguments) but to no avail either).

I'm not sure what to do now...

dickmao commented 4 years ago

The 25.1 issue is readily resolved. The larger issue is why ob-ein breaks so many org conventions that (org-babel-remove-result) was deemed necessary in the first place. No other ob- modules do this. No other ob- modules depend on the "drawer" keyword.

org-babel-insert-result defaults to "replace" the old result, so why do we explicitly need to org-babel-remove-result before inserting? The original authors of ob-ein, myself included, are not experienced org users and I likely need to rewrite the results logic.

yatsky commented 4 years ago

Thanks @dickmao . Could you please explain more about the 25.1 issue? I'm not sure what you meant by "is readily resolved" as the code hasn't been updated. Thank you.

dickmao commented 4 years ago

The 25.1 issue can be readily resolved by using one argument if major-version is less than 26, and two if not. Instead though, I've removed the call to org-babel-remove-result so I hope that resolves your issue.

Your issue has led me to rectify a basic misunderstanding about org-babel, the details of which are in 42134ad. Such stunningly basic failures really make me question, as in the past, whether ob-ein is useful to anyone.

yatsky commented 4 years ago

Thanks for the reply. I guess my real question was why condition-case was not working but just didn't make it clear.

I'm glad that this issue has helped in making EIN better. I really love it and really appreciate the effort you guys are putting into it. Now that you mentioned it and after reading @millejoh 's reply , I totally forgot why I always use ein-python instead of just ob-python since it seems what I wanted can be easily achieved by just using ob-python.

dickmao commented 4 years ago

The 25.1 error is a compilation error. Your condition-case would likely work at run-time, but the tests require clean compilation.