clojisr icon indicating copy to clipboard operation
clojisr copied to clipboard

cljdoc build failed

Open behrica opened this issue 1 year ago • 16 comments

I added cljdoc badge for Clojisr. Build is failing: https://cljdoc.org/builds/78095

Not sure, if easy fix

behrica avatar May 07 '24 08:05 behrica

cljdoc tries to load a namespace and run a R session

INFO: [:clojisr.v1.session/make-session {:action :new-session, :id nil, :actual-session-args {:session-type :rserve}}]
{:clojure.main/message
 "Execution error (AssertionError) at clojisr.v1.impl.rserve.proc/r-path (proc.clj:30).\nAssert failed: (not= % \"\")\n",
 :clojure.main/triage
 {:clojure.error/class java.lang.AssertionError,
  :clojure.error/line 30,
  :clojure.error/cause "Assert failed: (not= % \"\")",
  :clojure.error/symbol clojisr.v1.impl.rserve.proc/r-path,
  :clojure.error/source "proc.clj",
  :clojure.error/phase :execution},
 :clojure.main/trace

genmeblog avatar May 07 '24 08:05 genmeblog

Yes, noticed it. This line https://github.com/scicloj/clojisr/blob/82b4b77e53cc938c95694f19512c6b0187de3a5b/src/clojisr/v1/applications/plotting.clj#L14

fails on CI, correct ?

behrica avatar May 07 '24 09:05 behrica

I will try to add "no-doc" metadata: https://github.com/cljdoc/cljdoc/blob/master/doc/userguide/for-library-authors.adoc#declaring-your-public-api Not sure, if it prevents ns loading.

behrica avatar May 07 '24 09:05 behrica

It would only be visible on a new release, I believe. cljdocs build from maven jars, not from source. I propose we wait for next release and see if the cljdoc build works.

Its a nice way to have clojure docs for the API, so we should support it, I think

behrica avatar May 07 '24 09:05 behrica

Yes, it fails on CI. There is no R in the environment. clojisr.v1.r also will try to run a backend.

https://github.com/scicloj/clojisr/blob/82b4b77e53cc938c95694f19512c6b0187de3a5b/src/clojisr/v1/r.clj#L127

genmeblog avatar May 07 '24 09:05 genmeblog

I thought so....

behrica avatar May 07 '24 09:05 behrica

Maybe its overkill just to solve this issue, but did you ever try to do these "def" lazy and via "intern"..

behrica avatar May 07 '24 09:05 behrica

We were working on new session management system to solve this issue. But never finished. Maybe there is a way to refactor current setup without changing the API but I don't know it now. Any concrete ideas?

genmeblog avatar May 07 '24 09:05 genmeblog

I got this working:

(defn r== [e1 e2] ((clojisr.v1.r/r "`==`") e1 e2))

This would avoid the def and so the ns loading should "do nothing". Not sure, this can be done for "all" def in the ns.

behrica avatar May 07 '24 09:05 behrica

This way every call (r "==") will be reevaluated, RObject created and so on.

Anyway this is very good idea and I like it. We also get a function instead of var.

genmeblog avatar May 07 '24 09:05 genmeblog

I tried as well for r$, works in same way. Are all these defs about "binary" functions ? Or as well functions taking any number of args ?

behrica avatar May 07 '24 09:05 behrica

I don't know now. We have to go one by one. This strategy can be used also for plotting by replacing all RObjects calls into a string representation.

r.grDevices/dev-off -> "dev.off()" etc...

genmeblog avatar May 07 '24 09:05 genmeblog

Not sure, it can fully work for Clojisr, but it would follow a "clojure principle" that ns loading should not do "anything".

behrica avatar May 07 '24 09:05 behrica

I make a new issue at least , and link to this one.

behrica avatar May 07 '24 10:05 behrica

I don't know now. We have to go one by one. This strategy can be used also for plotting by replacing all RObjects calls into a string representation.

r.grDevices/dev-off -> "dev.off()" etc...

Would this be needed ? Or just do the " (require-r '[grDevices]) " later , "inside" the plot function ? Maybe with a delay to avoid repeated calls ?

behrica avatar May 07 '24 10:05 behrica

I don't know now. We have to go one by one. This strategy can be used also for plotting by replacing all RObjects calls into a string representation. r.grDevices/dev-off -> "dev.off()" etc...

Would this be needed ? Or just do the " (require-r '[grDevices]) " later , "inside" the plot function ? Maybe with a delay to avoid repeated calls ?

Yes, this is needed because later the symbol interned by require-r is used.

genmeblog avatar May 07 '24 10:05 genmeblog