Skip to content

Add optional convert options for r->clj functions #105

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.

Sign up for GitHub

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

Open
behrica wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add optional convert options for r->clj functions #105

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1354051
get all test work
behrica May 12, 2024
391b58c
add optional conversion params
behrica May 12, 2024
8ba38b3
ignore .calva files
behrica May 12, 2024
de1d7a5
Merge branch 'master' into addConvertOptions
behrica May 27, 2024
c0c7585
Merge branch 'master' of github.com:scicloj/clojisr into addConvertOp…
behrica May 27, 2024
93faa31
fixed options passig
behrica Sep 23, 2024
79866c2
first working conversion
behrica Sep 23, 2024
38c0d9a
wip - debug
behrica Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .devcontainer/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ FROM mcr.microsoft.com/devcontainers/java:11-bullseye

RUN bash -c "bash < <(curl -s https://raw.githubusercontent.com/babashka/babashka/master/install)"
RUN bash -c "bash < <(curl -s https://raw.githubusercontent.com/clojure-lsp/clojure-lsp/master/install)"
RUN apt-get update && apt-get install -y rlwrap gfortran libblas-dev liblapack-dev libpng-dev libfontconfig1-dev libfreetype-dev gfortran libicu-dev cmake
RUN apt-get update && apt-get install -y rlwrap gfortran libblas-dev liblapack-dev libpng-dev libfontconfig1-dev libfreetype-dev gfortran libicu-dev cmake leiningen libxi6 libxtst6
1 change: 1 addition & 0 deletions deps.edn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@
:test {:extra-paths ["test"]
:extra-deps {io.github.cognitect-labs/test-runner
{:git/tag "v0.5.0" :git/sha "b3fd0d2"}}
:jvm-opts ["-Djava.awt.headless=true"]
:main-opts ["-m" "cognitect.test-runner"]
:exec-fn cognitect.test-runner.api/test}}}
2 changes: 1 addition & 1 deletion src/clojisr/v1/impl/java_to_clj.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@

(defn java->clj
"Perform high level data conversion"
[exp]
[exp & options]
(let [exp (first-step->java exp)]
(cond
(data-frame? exp) (data-frame->dataset exp)
Expand Down
14 changes: 10 additions & 4 deletions src/clojisr/v1/r.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@
(let [session (session/fetch-or-make session-args)]
(prot/eval-r->java session r-code)))

(defn r->java [r-object]
(using-sessions/r->java r-object))
(defn r->java [r-object & options]
(using-sessions/r->java
(assoc r-object :options options)
))

(defn java->r [java-object & {:keys [session-args]}]
(let [session (session/fetch-or-make session-args)]
Expand All @@ -40,7 +42,7 @@
(defn java->native-clj [java-object]
(java2clj/java->native java-object))

(defn java->clj [java-object] (java2clj/java->clj java-object))
(defn java->clj [java-object & options] (java2clj/java->clj java-object options))

(defn clj->java [clj-object & {:keys [session-args]}]
(let [session (session/fetch-or-make session-args)]
Expand All @@ -49,7 +51,11 @@
(def clj->java->r (comp java->r clj->java))
(def clj->r clj->java->r)

(defn r->java->clj [r-object] (-> r-object r r->java java2clj/java->clj))
(defn r->java->clj [r-object & options]
(-> r-object
(r options)
Copy link
Member

@genmeblog genmeblog Sep 22, 2024

Choose a reason for hiding this comment

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

Why r should know about options?

Copy link
Member Author

@behrica behrica Sep 22, 2024

Choose a reason for hiding this comment

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

hmm. Not sure any more. Was some time ago.
I think the idea was to "be able" to pass options through all layers.

Copy link
Member

@genmeblog genmeblog Sep 23, 2024

Choose a reason for hiding this comment

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

Options should be passed only to the java->clj function.

Multidimensional conversion is done here: https://github.com/scicloj/clojisr/blob/master/src/clojisr/v1/impl/java_to_clj.clj#L94-L100

(r->java options)
(java2clj/java->clj options)))
(def r->clj r->java->clj)

(defn r->java->native-clj [r-object] (-> r r-object r->java java2clj/java->native))
Copy link
Member Author

@behrica behrica May 12, 2024

Choose a reason for hiding this comment

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

I did not change this function to take options.
Could not find any usage, neither sucessfully call it myself.

Expand Down
24 changes: 24 additions & 0 deletions test/clojisr/v1/matrix_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
(ns clojisr.v1.matrix-test
(:require
[clojure.test :refer [deftest is]]
[clojisr.v1.r :as r]))

(r/require-r '[base])

(deftest options-are-optional-without

(is (= (range 5)
(->
(r.base/matrix (range 5))
(r/r->clj)
(get 1)))))


(deftest options-are-optional-wit
;; does not crash
(is (= (range 5)
(->
(r.base/matrix (range 5))
(r/r->clj {:some-option true})
(get 1)))))