Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cljdoc build failed #94

Open
behrica opened this issue May 7, 2024 · 16 comments
Open

cljdoc build failed #94

behrica opened this issue May 7, 2024 · 16 comments

Comments

@behrica
Copy link
Member

behrica commented May 7, 2024

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

Not sure, if easy fix

@genmeblog
Copy link
Member

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

@behrica
Copy link
Member Author

behrica commented May 7, 2024

Yes, noticed it.
This line

(require-r '[grDevices])

fails on CI, correct ?

@behrica
Copy link
Member Author

behrica commented May 7, 2024

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
Copy link
Member Author

behrica commented May 7, 2024

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

@genmeblog
Copy link
Member

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

(def r== (r "`==`"))

@behrica
Copy link
Member Author

behrica commented May 7, 2024

I thought so....

@behrica
Copy link
Member Author

behrica commented May 7, 2024

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

@genmeblog
Copy link
Member

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?

@behrica
Copy link
Member Author

behrica commented May 7, 2024

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.

@genmeblog
Copy link
Member

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.

@behrica
Copy link
Member Author

behrica commented May 7, 2024

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 ?

@genmeblog
Copy link
Member

genmeblog commented May 7, 2024

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...

@behrica
Copy link
Member Author

behrica commented May 7, 2024

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

@behrica
Copy link
Member Author

behrica commented May 7, 2024

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

@behrica
Copy link
Member Author

behrica commented May 7, 2024

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 ?

@genmeblog
Copy link
Member

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.

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

No branches or pull requests

2 participants