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

Fix class not resolved in param declaration #47

Closed
VasLem opened this issue Oct 31, 2024 · 11 comments
Closed

Fix class not resolved in param declaration #47

VasLem opened this issue Oct 31, 2024 · 11 comments
Milestone

Comments

@VasLem
Copy link

VasLem commented Oct 31, 2024

How to deal with "undefined" variables, that are loaded by nextflow.config or exist in the environment?

image

Is there a way to silence these errors? If so it would be nice to have a quick fix above for ignoring that error at a line basis.

@bentsherman
Copy link
Member

Implicit environment variables like this will not be supported in the future because it is impossible to tell between an environment variable and a regular variable. We may add some alternative syntax in the future, but for now you can do System.getenv(REF_ROOT) to avoid the error

https://nextflow.io/docs/latest/vscode.html#excluded-syntax

@bentsherman bentsherman transferred this issue from nextflow-io/vscode-language-nextflow Oct 31, 2024
@VasLem
Copy link
Author

VasLem commented Oct 31, 2024

Alright, thanks, I see, I tried to make this change, but I receive a "System is not defined" issue now.

image

This error appears only for "System" commands called outside workflow and process context, it seems it works okay otherwise.
I understand that silencing an error is not a good practice if you are planning to have a strictly formal language specification. Also, I was not aware of that page you referenced, it clears out several issues I had regarding Groovy and Nextflow compliance.

@VasLem
Copy link
Author

VasLem commented Oct 31, 2024

Another thing popped up after I implemented the change, when I define an env var within a nextflow.config profile, it doesn't seem to be picked up by System.getenv within a process, I am left with null...

@bentsherman
Copy link
Member

I am seeing the issue with System.getenv in a param declaration, I will look into it

Regarding the env config scope, those variables are exposed to the task environment, not the pipeline script

@VasLem
Copy link
Author

VasLem commented Nov 1, 2024

The thing is that until now, the env vars were picked just fine by the script as well... Wasn't this not intended? If I just leave the variables as is, without using System.getenv, they work perfectly fine, with the right value given in the config.

@bentsherman
Copy link
Member

Interesting, that is not the intention, see the relevant docs here:

https://nextflow.io/docs/latest/reference/config.html#env

Can you share a minimal test case where an env in the config is printed in the script?

@bentsherman
Copy link
Member

Actually it was pretty easy to replicate:

env {
    FOO = 'bar!'
}
println "${FOO}"
$ nextflow run test.nf

 N E X T F L O W   ~  version 24.10.0

Launching `test.nf` [shrivelled_almeida] DSL2 - revision: 320593a247

bar!

@pditommaso FYI, it looks like config env settings are exposed in the script?

@VasLem
Copy link
Author

VasLem commented Nov 1, 2024

(...it would be great to have this non-intended behavior stay as is, changing profiles and having env variables adapt to these changes is really, really helpful 🥹)

@pditommaso
Copy link
Member

FYI, it looks like config env settings are exposed in the script?

Not 100% sure it was desired. We may need to review along with the mechanism to access env variables both in config and script files.

Maybe, it could be used env. both to define and access env variables both for tasks and script

@bentsherman
Copy link
Member

Maybe, it could be used env. both to define and access env variables both for tasks and script

I was thinking the same, env() would be nicer than System.getenv() and consistent with env() process output

@bentsherman bentsherman added this to the v1.0.2 milestone Nov 15, 2024
@bentsherman bentsherman changed the title Dealing with environment variables Fix class not resolved in param declaration Nov 15, 2024
@bentsherman
Copy link
Member

The issue with System.getenv() will be fixed in the next patch release. Also, a new env() function will be introduced in Nextflow 24.11.0-edge

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

3 participants