scicloj / clojisr

Clojure speaks statistics - a bridge between Clojure to R
https://scicloj.github.io/clojisr/
Eclipse Public License 2.0
147 stars 9 forks source link

proof of concept for attaching :doc metadata to generate functions #98

Open behrica opened 1 month ago

behrica commented 1 month ago

This is very draft. I was not able to re-use the existing "help" , and need a hack to have a session. And on this I am not sure, how to do better.

behrica commented 1 month ago

@genmeblog, maybe you can improve on it.

behrica commented 1 month ago

I removed the duplication of the help logic. Not 100% sure, if I pass the "session" around correctly. But it does work now.

BUT its slow. It increases quite a lot the time of require-r, not sure if something can be done. I noticed as well that "Calva" is not able to render tgis type o dynamic ":doc". Emacs does, and repl shows them as well yuing (doc ...)

behrica commented 1 month ago

Can be used like this:

(reset! clojisr.v1.require/attach-help-as-docstring-to-vars true)
(r/require-r '[ggplot2])
(r/require-r '[ranger])
(r/require-r '[base])

Help generation is as well memoized, so at least fast on second call

behrica commented 1 month ago

It does work as well for Calva, not sure why it failed as I tried.

genmeblog commented 1 month ago

Maybe instead of attach-help-as-docstring-to-vars add an option to require-r itself?

(require-r '[ggplot :docs? true])

behrica commented 1 month ago

Maybe instead of attach-help-as-docstring-to-vars add an option to require-r itself?

(require-r '[ggplot :docs? true])

Yes, I thought about it. Not sure, what best.

Ideally we could speed it up..., but likely not possible. To mee it something you "either want" or "don't want".

Not per library.

genmeblog commented 1 month ago

Yes, you're right, so maybe an argument to require-r itself?

behrica commented 1 month ago

FYI: Currently there is a bug in VSCode / Calva which prevents the rendering of this help attached to the vars. https://github.com/BetterThanTomorrow/calva/issues/2552 But it seems fixable, as Calva has already foreseen to take doc from Cider and not only from LSP. LSP is not informed about such dynamic vars, so cannot report this doc strings.

behrica commented 1 month ago

In my view the "slow R package loading" due to help loading is acceptable, as it happens only once per R package, then its cached. So maybe an atom to switch it on/off is enough. Or we make it "delayed", So we do an "immediate loading" without help, and the we load again on an other thread in a background.

genmeblog commented 1 month ago

We can also alter vars in the background I think.

behrica commented 1 month ago

I am pretty happy with it now.

The :doc gets now changed to be "help" via alter-meta! in a future, always. This is transparent for the user in most cases, as in repl the require returns immediately. and a few seconds later, the doc string of the var is updated, and IDEs should show it.

behrica commented 1 month ago

Remark: as we use now a "future" we get the 60 seconds waiting when clojure shuts down

genmeblog commented 1 month ago

Remark: as we use now a "future" we get the 60 seconds waiting when clojure shuts down

Strange, futures shouldn't affect the shutdown time...

behrica commented 1 month ago

Remark: as we use now a "future" we get the 60 seconds waiting when clojure shuts down

Strange, futures shouldn't affect the shutdown time...

Yes, they do. https://clojure.atlassian.net/browse/CLJ-124

genmeblog commented 1 month ago

Ok. I see. What about add a (shutdown-agents) here: https://github.com/scicloj/clojisr/blob/master/src/clojisr/v1/r.clj#L201 ?

behrica commented 1 month ago

Ok. I see. What about add a (shutdown-agents) here: https://github.com/scicloj/clojisr/blob/master/src/clojisr/v1/r.clj#L201 ?

Will not make a difference, as this hook is called "too late as well" , I suppose. I will research if some other "parallel executors" do not have this issue.

genmeblog commented 1 month ago

When does this 60s delay happen? I mean what cases are affected?

behrica commented 1 month ago

When does this 60s delay happen? I mean what cases are affected?

In practice only in units tests...or when running "clj -e ' ' ". In interactive REPL work, the JVM is up for hours, and "exiting" it kills the daemon threads. So "scripts" could be affected, not sure anybody doe stehm with ClojisR.

behrica commented 1 month ago
time clj -e "(require '[clojisr.v1.r :as r])(r/require-r '[utils])"

real    1m10.797s
user    0m29.738s
sys     0m0.844s

Here we see the 60 seconds delay.

vs

time clj -e "(require '[clojisr.v1.r :as r])(r/require-r '[utils])(System/exit 0)"
real    0m14.728s
user    0m29.386s
sys     0m0.827s

same duration as:

time clj -e "(require '[clojisr.v1.r :as r])(r/require-r '[utils])(shutdown-agents)"

real    0m15.212s
user    0m29.055s
sys     0m0.767s

So people doing scripts just need to remember to add it.