Hi all, I'm a software engineer at Seqera, and formerly a bioinformatician, and I made a small issue and PR of 162 lines of syntax change to drop the number of nextflow strict syntax warnings to 0. It was AI-assisted using our new agent, but human-reviewed, and end to end tested.

Hi Nico, could you join the team in #CEB982K2T so that there is no need to approve for the CI to run on any on your I hope many future PRs

I approve it, but make a small suggestion for the changelog message

I made a suggestion on Jose's suggestion

Thanks for the quick feedback

Thanks, I c/p it and did not modify the number 😓

I decided to scope that to a larger upcoming issue

For the newer PR, it would be nice that they are as atomic as possible

Roger that. Thanks for the feedback

I wanted to update #CE5EL6326 but did not have the time

Absolutely, makes sense. Now that there's a opinionated linter with strict syntax this is a perfectly suited low-hanging fruit for agentic engineering, so I'm using this as a proof of concept, but will still be the human-in-the-loop.

If you want to work on more pipeline, we always welcome people in sarek

😂 the sarek hunting game.

auxd35.jpg

I have made a follow-up issue to update nf-core modules: by doing so, the number of strict syntax warnings drops to 0. https://github.com/nf-core/atacseq/issues/447

@Nico De Veaux, I think Jose is on vacation. I had a look at your PR #453 could you also update the changelog? Then it should be good to go

Thanks, updated it

I think there was a warning for the newest nextflow version tests, potentially we should dig into it, this week is a bit chaotic on my end...

I see that now, taking a look at what might resolve that. There's also a warning about "Newer version of the nf-core template is available."

Thank you for looking into it!

I did a re-run, and it succeeded (at 17:53 CET), so it's a flaky test.

Looks good now. I like the sorting idea, even though it adds a bit of overhead.

Alright! Now that the template merge is in, it unblocks the big

nf-core module update --all

PR I made: github.com/nf-core/atacseq/pull/448.

Nico you are on fire 😉 I'll look at it later, the morning is a bit tight already.

Sorry for the radio silence, I was on break, but I'm addressing these changes today.

Mhmm, I think you could check which samples are used for the full-size dataset test ran upon release. But before doing those large scale computations, how do the test snapshots behave? Do you spot obvious problems?

No problems on the test snapshots. The only obvious problem is that subread is a newly imported module, and subread changed its behavior in 2.1.

Hi Nico, I really like your enthusiasm 🙂 But I think we should stick to a more atomic development / PR strategy. It get's pretty complex and difficult to review ~500 changed files. I'm getting a bit confused 😉 #448 is the modules update? Which was good so far, only the changelog updates were missing, right? #452 is the paired-end and single-end BAM handling? But why are there >200 files changed, is that a copy of #448 plus some additional changes? Tbh, allowing the handling of SE/PE experiments in the same run is a big conceptional change and something we should discuss in a bigger cycle. You may introduce batch effects when doing DE analysis of SE and PE data together, starting with read aggregation (counting both reads of a PE experiment or only a given fragment once). But this actually has an even bigger impact... e.g on PCR deduplication or computing coverage tracks, where SE reads can be shifted by a theoretical fragment length, while in PE data you can use the center of the fragment. Personally, I feel like we should keep SE/PE data separated and don't allow for mixed library types but this would be something up for discussion. Haven't checked the pipeline, not sure what is currently allowed. What about the following: I will check your changes in #448 and merge it. You raise an issue for #452 and we wait for @Jose Espinosa-Carrasco feedback? E.g could be discussed in #C0C30LAV0G7 and then we get more feedback in the general channel?

Thanks for the feedback. That makes sense. And yes, 452 is big because it has 448 merged in.

Perfect! I just approved #448, feel free to merge