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

Initial Release Request #14

Merged
merged 58 commits into from
Nov 14, 2023
Merged

Initial Release Request #14

merged 58 commits into from
Nov 14, 2023

Conversation

cmatKhan
Copy link
Collaborator

@cmatKhan cmatKhan commented Nov 1, 2023

This represents the proposed initial release of callingcards

Updated testing; adding yeast workflow
CHANGELOG.md Outdated Show resolved Hide resolved
assets/samplesheet.csv Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these files should be in the pipeline

Copy link
Collaborator Author

@cmatKhan cmatKhan Nov 2, 2023

Choose a reason for hiding this comment

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

I consulted one of the wet lab members who is more familiar with the biological details -- I'll include his response below.

But, briefly, the yeast strain that makes the 'callingcards' work to begin with is engineered. At the moment, all callingcards experiments would need to use these masks and plasmid sequences (adjusted for the genome build). These are used in the default_yeast profile, which also specifies the ignomes sacCer genome, and allows the yeast workflow to run by only providing the samplesheet. Thus, I think it is reasonable to distribute these along with the workflow.

Here is the more detailed version:

The plasmids used to perform the yeast calling card assay contain sequences from the yeast genome. This allows the plasmids to be selected, propagated, and be generally useful as tools in yeast. However, the Ty5 transposable element used to mark TF-binding can also insert into plasmid-borne sequences (Liu et al, 2020 PMID: 32133534). Because the plasmids carry small sequences from the genome, the number of Ty5 insertions mapped to these sequences could be artificially high, simply because there are more molecular copies of these sequences in the nucleus (one copy of each sequence in the haploid genome and 5-10 copies of each sequence on a low-copy plasmid or 50 -100 copies of each sequence on a 2micron plasmid). This could give a false impression of how well the transcription factor binds these sequences. Also, a collection of 50 barcoded Ty5 plasmids, as well as many of the transcription factor-carrying plasmids have already been cloned and are available to the community. This means someone doing the yeast calling card assay for the first time would likely use these available plasmids if possible, rather than clone their own plasmids. So, I think it is reasonable for the pipeline to include masking these regions as an option.

Copy link
Member

Choose a reason for hiding this comment

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

the 90° corner looks weird with the 45° ones, can we have just one of these 2 possibilities?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this in reference to the labels, like "Process Reads"? If not, which corners?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I rounded the label corners that were not rounded in 5fa1c19. it does look better that way. If there were other corners, let me know.

main.nf Show resolved Hide resolved
Comment on lines +1 to +5
reports:
multiqc_report.html:
display: "MultiQC HTML report"
samplesheet.csv:
display: "Auto-created samplesheet with collated metadata and FASTQ paths"
Copy link
Member

Choose a reason for hiding this comment

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

Can you details the simple files created by the pipeline there?

Copy link
Collaborator Author

@cmatKhan cmatKhan Nov 2, 2023

Choose a reason for hiding this comment

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

resolved (i believe) in b326514. I'm not a tower user -- if you (or someone) wouldn't mind glancing at that, I'd appreciate it.

cmatKhan and others added 5 commits November 2, 2023 10:18
update conda/quay paths in bwa_aln_to_bam/main.nf

Co-authored-by: Maxime U Garcia <[email protected]>
…lesheet, error handling in main and tower output config
merging upstream/dev after making review changes
@FriederikeHanssen
Copy link
Contributor

Hey! Since the master branch is not completely clean, I will merge this PR and create a new Pseudo PR from the current commit to the first one.

@FriederikeHanssen FriederikeHanssen merged commit 1d236e4 into master Nov 14, 2023
13 checks passed
Copy link

github-actions bot commented Nov 28, 2023

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 316a52e

+| ✅ 157 tests passed       |+
!| ❗   4 tests had warnings |!

❗ Test warnings:

  • nextflow_config - Config manifest.version should end in dev: 1.0.0
  • readme - README contains the placeholder zenodo.XXXXXXX. This should be replaced with the zenodo doi (after the first release).
  • pipeline_todos - TODO string in README.md: Add citation for pipeline after first release. Uncomment lines below and update Zenodo doi and badge at the top of this file.
  • pipeline_todos - TODO string in WorkflowMain.groovy: Add Zenodo DOI for pipeline after first release

✅ Tests passed:

Run details

  • nf-core/tools version 2.10
  • Run at 2023-11-03 20:08:17

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.

3 participants