search

stefan-m-lenz / JuliaConnectoR

A functionally oriented interface for calling Julia from R
Other
100 stars 6 forks source link

Dev julia opts #11

Closed bakaburg1 closed 2 years ago

bakaburg1 commented 2 years ago

Implemented the possibility to pass options at Julia startup

stefan-m-lenz commented 2 years ago

This looks very complicated. Is there a reason why can't you simply allow the user to set e.g. 

Sys.setenv("JULIACONNECTOR_JULIAARGS" = "-O 3 -J sysimg")

and why you need the function setJuliaStartupArgs? Everything added to the package must be maintained. In order to do this, thorough automatic testing must be implemented. Added functionality must be minimal. I would rather not have a function setJuliaStartupArgs for setting an environment variable if it is not needed. (Also, everything must be documented. )

stefan-m-lenz commented 2 years ago

I think checking the arguments is not necessary. If a user gives a wrong arguments, Julia startup will fail and the Julia error will be displayed to the user.

bakaburg1 commented 2 years ago

I think checking the arguments is not necessary. If a user gives a wrong arguments, Julia startup will fail and the Julia error will be displayed to the user.

Ok I just wanted to be sure to not allow users to break things unconsciously or receive hard to interpret errors. If you think is overkill I can remove it.

Are single dash arguments not allowed?

If I understood Julia docs well (https://docs.julialang.org/en/v1/manual/getting-started/), if you have startup options then you need to put a double dash before passing files, with the following pattern: julia [switches] -- [programfile] [args...] Should I remove it?

stefan-m-lenz commented 2 years ago

If I understood Julia docs well (https://docs.julialang.org/en/v1/manual/getting-started/), if you have startup options then you need to put a double dash before passing files, with the following pattern: julia [switches] -- [programfile] [args...] Should I remove it?

Ah OK, now I understand. This is something that should be documented.

bakaburg1 commented 2 years ago

ok, so I remove startupArgs e leave the double dashes? BTW in the last commit I changed "args" to "opts" in the whole code, since "arguments" wasn't the correct term. do you agree with that?

stefan-m-lenz commented 2 years ago

ok, so I remove startupArgs e leave the double dashes? BTW in the last commit I changed "args" to "opts" in the whole code, since "arguments" wasn't the correct term. do you agree with that?

I agree, OPTS is better! :thumbsup

stefan-m-lenz commented 2 years ago

Ok I just wanted to be sure to not allow users to break things unconsciously or receive hard to interpret errors. If you think is overkill I can remove it.

Yes, I think it is overkill. If a user sets a command line argument and Julia gives an error when starting up, the user will now that he/she messed up the arguments. Also the error is displayed (after the timeout). The disadvantage of checking the arguments yourself is that you have to test this implementation throughly and handle all the possible cases. I think this is too much work for an "expert feature" like this.

bakaburg1 commented 2 years ago

I removed the functions to interact with the startup options. Do you need me to edit anything else? like documentation or tests?

stefan-m-lenz commented 2 years ago

Thanks, I will think about a good test and add it next week.