[DO NOT MERGE] Re-worked tooling for Python environments in REMIND

[tonnrueter]

Please do not merge this PR since it breaks API I still need some time to make some changes to REMIND that use these new functions

NEW

• archivePythonEnv.R: Create a file listing all Python packages installed in the provided Python environment
• checkPythonEnv.R: Determines the location of interpreter binary in REMINDs Python environment. Works with system Python as well as with venv/conda based virtual Python environments
• checkPythonRequirements.R: Checks if external Python dependencies are present in the Python environment
• checkPythonDeps.R: Check if external Python dependencies can actually be imported. Intended to be used before starting a REMIND run

REMOVED

• pythonBinPath.R: This replaced by checkPythonEnv.R
• writePythonVirtualEnvLockFile.R/createResultsfolderPythonVirtualEnv.R: This was absorbed into archivePythonEnv.R
• updatePythonVirtualEnv.R: Do not allow manipulation of the REMIND Python environment from REMIND

[pascal-sauer]

You can mark a PR as draft (which I just did) so it cannot be merged :)

[pascal-sauer]

Impressive work! Some of the problems you solve here I suspect have already been solved in other places (R's numeric_version, python modules) that we should maybe use instead as they have been more thoroughly tested by the community.

[pascal-sauer]

This is a lot of code duplication. What about something like this:

f <- switch(action,
  stop = stop,
  warn = warning,
  note = message,
  pass = function(...) TRUE
)

And then you only need the block once and instead of e.g. stop you call f?

[tonnrueter]

I like the idea, however if the stop/warn messaging is moved to a dedicated function (let's call it notifyUser), the origin of the error is obfuscated, e.g.:

anotherFunction <- function() {
  [..do things, error occurs..]
  notifyUser("stop", "foobar")
}

yields Error in notifyUser("stop", "error message") : error message instead of Error in anotherFunction [...].

I consolidated the error message creation now

[pascal-sauer]

What a shame, and it seems stop does not have a way to include the previous call on the call stack in the error message instead :/

[pascal-sauer]

This again feels like it should already exist in a python module. Writing this kind of code from scratch seems very risky to me.

[tonnrueter]

You're right, Python can do this, however we need R to do it, too 😬 This function is called before we can rely on using Python in REMIND

[pascal-sauer]

<<- should really be avoided if at all possible. Is it possible to return what you want to add here and then add it in a scope where missingDependencies is accessible?

[tonnrueter]

I was not able to find another way to access missingDependencies form the error callback. I'd argue that this is the exception to the (otherwise perfectly fine) rule to avoid <<-, as the error case is two-fold: We're not only interested in dependencies that cannot be imported (which would trigger the error callback) but also a version mismatch (strict mode), where the dependency can be imported but has a version different from what's listed in the requirements.

I wanna respect your best R-practices here: If you strictly oppose <<--usage, I'll isolate the whole try-catch-block in a separate function that returns NULL if everything's fine, else returns a string giving a reason

[pascal-sauer]

I do strictly oppose <<-, but I want to stress this is not "my" best R-practice 😉

[tonnrueter]

Imma make it a function then :)

I learned a lot about implicit return values of the tryCatch-statement in R and found a way to remove <<- 👍

[pascal-sauer]

See my suggestion further below