Skip to content
This repository has been archived by the owner on Nov 26, 2021. It is now read-only.

Add session async for ruby #2

Merged
merged 8 commits into from
Oct 17, 2019
Merged

Add session async for ruby #2

merged 8 commits into from
Oct 17, 2019

Conversation

suonlight
Copy link
Contributor

No description provided.

Copy link

@nico202 nico202 left a comment

Choose a reason for hiding this comment

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

There are references to R

lisp/ob-session-async-ruby.el Outdated Show resolved Hide resolved
lisp/ob-session-async-ruby.el Outdated Show resolved Hide resolved
lisp/ob-session-async-ruby.el Outdated Show resolved Hide resolved
@jackkamm
Copy link
Owner

This looks great! Sorry for the delay -- I'm just seeing this now, my notifications settings must have gotten messed up.

I'll need to take a little time to look at this (I've never used Ruby) but wanted you to know it's on my radar now.

If you get a chance, it would be very helpful if you could also add a simple unit test in test/ob-session-async-test.el.

@suonlight
Copy link
Contributor Author

@jackkamm The current tests are not passed on my local before this PR. I don't know why:

executing R code block...
ess-tracebug mode enabled
Starting evaluation...
Eval buffer
Code block evaluation complete.
Code block evaluation complete.

executing R code block...
Starting evaluation...
Eval buffer
Starting evaluation...
Eval buffer
Code block evaluation complete.
.Test ob-session-async-R-named-value backtrace:


Test ob-session-async-R-named-value condition:

    (ert-test-failed
     ((should
       (progn
         (org-babel-execute-src-block)
         (sleep-for 0 200)
         (string= ... ...)))
      :form
      (progn
        (org-babel-execute-src-block)
        (sleep-for 0 200)
        (string=
         (concat src-block results-after)
         (buffer-string)))
      :value nil))


executing R code block...
Starting evaluation...
Eval buffer
Code block evaluation complete.
Code block evaluation complete.

executing R code block...
Starting evaluation...
Eval buffer
Code block evaluation complete.
Code block evaluation complete.

executing R code block...
Starting evaluation...
Eval buffer
Starting evaluation...
Eval buffer
Code block evaluation complete.
F..Test ob-session-async-R-simple-session-async-value backtrace:


Test ob-session-async-R-simple-session-async-value condition:

    (ert-test-failed
     ((should
       (let
           (...)
         (and ... ...)))
      :form
      (let
          ((expected "Yep!"))
        (and
         (not ...)
         (string= expected ...)))
      :value nil))


executing R code block...
Starting evaluation...
Eval buffer
Starting evaluation...
Eval buffer
Code block evaluation complete.
FTest ob-session-async-R-value-drawer backtrace:


Test ob-session-async-R-value-drawer condition:

    (ert-test-failed
     ((should
       (progn
         (org-babel-execute-src-block)
         (sleep-for 0 200)
         (string= ... ...)))
      :form
      (progn
        (org-babel-execute-src-block)
        (sleep-for 0 200)
        (string=
         (concat src-block result)
         (buffer-string)))
      :value nil))

F

Ran 6 tests in 2.908 seconds
3 unexpected results:
   FAILED  ob-session-async-R-named-value
   FAILED  ob-session-async-R-simple-session-async-value
   FAILED  ob-session-async-R-value-drawer

@jackkamm
Copy link
Owner

Interesting -- it looks like the tests with :results value are the ones failing, whereas the tests with :results output are fine.

Could you try adding (print (org-babel-read-result)) to the test ob-session-async-R-simple-session-async-value to see what might be going on?

Also -- what OS and emacs version are you using?

Thanks in advance, and sorry for the tests failing -- I think you're the first person besides myself to run them, so this is useful info for me.

@suonlight
Copy link
Contributor Author

suonlight commented Oct 14, 2019

I'm using:

OS: Mac OSX Mojave 10.14.5
Emacs: 27.0.50
Org-Mode: 9.2.5

The code always returns temp file even when I increases sleep-for
(print (org-babel-read-result));; /var/folders/_7/jvqjkfkj1p77dwlg23tl7d200000gn/T/R-kZawq4

@jackkamm
Copy link
Owner

Thanks for the info, I'm running on emacs 26 on Linux which explains the difference. I'll try to reproduce this on emacs 27/an old macbook I have lying around.

@jackkamm jackkamm mentioned this pull request Oct 14, 2019
@suonlight
Copy link
Contributor Author

If you use emacs 27, then you should add

(when (> emacs-major-version 26)
  (defalias 'ert--print-backtrace 'backtrace-to-string))

to test-helper. See rejeep/ert-runner.el#49 for more details.

@suonlight
Copy link
Contributor Author

@jackkamm I have added units for Ruby and fix failed unit tests for R. The problem is: in each test case, we need to add (org-babel-temporary-directory "/tmp") to generate temp file like /tmp/ruby-fRIHNK. Otherwise, it will generate the temp file like ob_comint_async_ruby_file_/var/folders/_7/jvqjkfkj1p77dwlg23tl7d200000gn/T/ruby-fRIHNK which failed the indicator because of _7.

Now, all tests for ruby and R work well.

Screenshot 2019-10-17 12 03 56

Pls help me review again.

@jackkamm
Copy link
Owner

jackkamm commented Oct 17, 2019

This is great, thanks. The code looks good, and I can confirm all the unit tests pass on my system.

Also, I very much appreciate you fixing the R unit tests for emacs27/macOS -- thanks!

As soon as you fix my 1 minor comment above (about authorship) I will merge this in.

I should also note, this PR achieves a major milestone for this project, which is to add support for a second language!

I am thinking I would like to add 1 more language, just to make sure the core architecture in ob-session-async.el is general enough. Then I will start considering whether to submit this functionality upstream into org-mode. If you any thoughts/comments on this I would be interested to hear your opinions.

@jackkamm jackkamm merged commit 578b923 into jackkamm:master Oct 17, 2019
@suonlight
Copy link
Contributor Author

@jackkamm Contributing to org-mode is great. I just have a concern about ob-async. This package and ob-async should merge into one before submitting to org-mode. But, we should add 1 more language first to see how the core architecture is good enough. In the meantime, we need a post on Reddit about this package again :) to get more users and insight

@jackkamm jackkamm mentioned this pull request Dec 28, 2019
@jackkamm
Copy link
Owner

@suonlight

Session async has been added upstream to Org since 9.5. In particular, the functionality in ob-session-async.el was moved into ob-comint.el, though slightly renamed (e.g., ob-session-async-register is now org-babel-comint-async-register). Implementations for Python and R were also added to Org mode.

Therefore, I am going to archive this repo.

I encourage you to submit your async Ruby implementation into Org mode, or to make a standalone package for it.

Thanks again for implementing async Ruby session evaluation!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants