-
Notifications
You must be signed in to change notification settings - Fork 764
Upgrade to Groovy 5 #6220
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?
Upgrade to Groovy 5 #6220
Conversation
✅ Deploy Preview for nextflow-docs-staging canceled.
|
|
Worth following this discussion, it looks like there are not negligile perf issue with Groovy 5 and invoke dynamic apache/grails-core#15293 |
|
Interesting, it looks similar to the issues they had with Groovy 4. It looks like one commenter avoided the perf issues by using |
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
✅ Deploy Preview for nextflow-docs-staging canceled.
|
| // ERROR: Cannot call java.lang.Integer#compareTo(java.lang.Integer) with arguments [java.lang.Object] | ||
| while( (line=reader.readLine()) && c++ < MAX_LINES ) { |
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 tried changing the type of c and MAX_LINES to Integer but I get this type checking error regardless. Seems like a Groovy type checker bug
| // ERROR: Cannot find matching static method nextflow.extension.CH#createBy(groovyx.gpars.dataflow.DataflowWriteChannel) | ||
| final target = CH.createBy(source) |
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.
Based on my limited knowledge of how the Groovy type checker does type inference with instanceof, it seems to be applying the instanceof check incorrectly, treating source as type DataflowWriteChannel through the entire method instead of just the if statement. Likely a Groovy bug
Initial attempt to upgrade to Groovy 5.
Benefits:
zip(can be added to Nextflow standard library)instanceof(can be added to Nextflow language)_for placeholder variables (can be added to Nextflow language)Changes:
Had to remove generic type arguments from
instanceofchecks as the Groovy parser doesn't allow it anymoreHad to add
@Slf4jto some inner classes to makelogwork in that class. not sure if this is a bug or just a breaking changeCurrent issues:
Groovy complained about a few implementers of
CacheFunnel, even though I didn't see anything wrong. Converting CacheFunnel to Java fixed the issue for one but not the otherMultiple issues with the type checker that I'm pretty sure are just Groovy bugs, particularly
CmdLogandDumpOpThe type checker does not recognize
targetinTaskConfigas a Map, seems like a bug. I wonder if it's an edge case sincetargetis defined in super class as private and a DelegateOverall it looks like Groovy 5 beta still has a few compiler bugs to sort out. But I'm excited to get it working so that I can bring the new pattern matching features into the Nextflow language.