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

Allow closure type as input in PublishDir.setMode() #4024

Closed
dwishsan opened this issue Jun 14, 2023 · 2 comments
Closed

Allow closure type as input in PublishDir.setMode() #4024

dwishsan opened this issue Jun 14, 2023 · 2 comments
Labels

Comments

@dwishsan
Copy link

Hi everyone !

First of all, a big THANK YOU for nextflow, it is very handy :)

Some context on my feature request

I am using nextflow with configuration files to customize processes.
This mean, among other things, that I DO NOT define the publishDir directive for each process, like what is done in the nextflow documentation, I rather define the publishDir {} one and one time only for all processes (in a process {} directive) using a closure for the publisDir.path.
This is very handy and it works very well!

But recently, I wanted to add the possibility to copy the published files instead of doing symlinks (the default). In theory, this is very easy to do, I just have to set publishDir.mode with the wanted closure.

But I get this error:
Invalid method invocation 'setMode' with arguments: (Script989C00012DEDEFE63A87A761299AE910$_run_closure6$_closure56) on PublishDir type

Indeed, in the code, PublishDir.setMode() can take either a nextflow.processor.PublishDir.Mode or a String, but not a closure.

New feature

Add a setMode(def value) method for the PublishDir class.

Usage scenario

I do not know if other individuals are using closure to set their publishDir.path, but it would be very helpfull for me if this feature is added in Nextflow :)

Suggest implementation

I THINK this should work:

void setMode( Closure value ) {
  setMode(value.call())
}

Best,
Audric

@dwishsan
Copy link
Author

I just realised that publishDir.enabled does not work with closures either. A workarround, suggested here: #3692, could be to set publishDir.path to null, but this is not allowed by Nextflow (Path string cannot be empty). This is due to the following piece of code:

void apply( Set<Path> files, TaskRun task ) {

        if( !files || !enabled )
            return

        if( !path )
            throw new IllegalStateException("Target path for directive publishDir cannot be null")

        if( nullPathWarn )
            log.warn "Process `$task.processor.name` publishDir path contains a variable with a null value"

        this.sourceDir = task.targetDir
        this.sourceFileSystem = sourceDir.fileSystem
        this.stageInMode = task.config.stageInMode
        this.taskName = task.name

        apply0(files)
    }

But it could be replaced by this in order to work, if there is not particular reason why the publishDir.path can not be null:

void apply( Set<Path> files, TaskRun task ) {

        if( !files || !enabled || !path)
            return

        if( nullPathWarn )
            log.warn "Process `$task.processor.name` publishDir path contains a variable with a null value"

        this.sourceDir = task.targetDir
        this.sourceFileSystem = sourceDir.fileSystem
        this.stageInMode = task.config.stageInMode
        this.taskName = task.name

        apply0(files)
    }

I was wondering: is there any reason for the publishDir directives (except path and saveAs) to not accept closures as input?
Shouldn't all of them accept closures?

@bentsherman bentsherman linked a pull request Jun 15, 2023 that will close this issue
@pditommaso
Copy link
Member

Thanks for the suggestion. No plan to support this in the short term. We are working to a better why to define publishDir defaults #4186

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

Successfully merging a pull request may close this issue.

2 participants