-
Notifications
You must be signed in to change notification settings - Fork 746
Change return type of listFiles() from Path[] to List<Path>
#6581
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Ben Sherman <[email protected]>
✅ Deploy Preview for nextflow-docs-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
This can introduce breaking changes in third parties pipelines. Not sure the change is worth |
|
The problem is that these methods are documented in the Nextflow language as returning a list instead of an array:
Because arrays don't exist in the Nextflow type system It looks like everyone is wrapping |
Signed-off-by: Ben Sherman <[email protected]>
pditommaso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the point about type system, however the array type has been used to keep aligned with the File version in Groovy SDK, until there isn't a clear boundary between nextflow and groovy and java API this change little.
Secondly and even this kind of changes are extremely annoying for users because may introduce subtle bugs hard to detect, and confuse agents.
If this change is really needed better to deprecated, document and induce and new API
@pditommaso what method are you talking about? I can't find anything in the Groovy or Java SDK that returns a path array |
Fix #6152
Functions in the standard library that return an array instead of a list are problematic because arrays don't have all the same functionality of a list. For example, a
Path[]can't be accepted by process path inputs which expect aCollection<Path>.Should be effectively non-breaking for Nextflow users, but let's see if it breaks anything in the runtime...