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

Dev -> Master for v3.10 release #914

Merged
merged 84 commits into from
Dec 21, 2022
Merged

Dev -> Master for v3.10 release #914

merged 84 commits into from
Dec 21, 2022

Conversation

drpatelh
Copy link
Member

No description provided.

nf-core-bot and others added 30 commits October 4, 2022 22:04
Update modules to latest restructuring on nf-core/modules
Important! Template update for nf-core/tools v2.6
Because I just found out about these and want to try them out somewhere.
Fancy GitHub blockquote admonitions
clarification of star and salmon outputs from issue #722
@github-actions
Copy link

github-actions bot commented Dec 20, 2022

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 4f29256

+| ✅ 153 tests passed       |+
#| ❔   3 tests were ignored |#
!| ❗   2 tests had warnings |!

❗ Test warnings:

  • nextflow_config - Config manifest.version should end in dev: '3.10'
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your prefered methods description, e.g. add publication citation for this pipeline

❔ Tests ignored:

  • files_unchanged - File ignored due to lint config: assets/email_template.html
  • files_unchanged - File ignored due to lint config: assets/email_template.txt
  • files_unchanged - File ignored due to lint config: lib/NfcoreTemplate.groovy

✅ Tests passed:

Run details

  • nf-core/tools version 2.7.2
  • Run at 2022-12-20 22:00:10

Bump pipeline version to v3.10 and other small tweaks
Copy link
Contributor

@robsyme robsyme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Monster merge, but I'm happy with all of the commits that comprised this PR. We're also running a full-sized test now, and will update here with the results.

CHANGELOG.md Outdated Show resolved Hide resolved
}
.groupTuple(by: [0])
.groupTuple()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about using sizehere so the tuple advances as soon as possible?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! I'll leave @robsyme to comment here. Asked him exactly the same question.

Copy link
Contributor

@robsyme robsyme Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question.
To emit asap, we'd need to calculate the number of technical replicates at an earlier stage - probably during samplesheet validation. This is certainly possible, but because there are no long-running processes between samplesheet generation and this grouping operation, there is no real speedup to be gained.
I'm a huge fan of using groupKey as often as possible, but in this case there is no advantage.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok makes sense.

Copy link
Contributor

@FriederikeHanssen FriederikeHanssen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good :) Maybe someone with a little more in depth knowledge of what the pipeline does should approve though :)

Copy link
Contributor

@FriederikeHanssen FriederikeHanssen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now! Not deep into rnaseq analysis, but I suppose the features were discussed beforehand on slack.

@drpatelh drpatelh merged commit adce7ce into master Dec 21, 2022
@robsyme
Copy link
Contributor

robsyme commented Dec 21, 2022

🥳

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

Successfully merging this pull request may close these issues.

8 participants