Solve some Singularity issues. Updates. DSL2 Migration. #57
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Additionally, changed README and use Nextflow`s default singularity conversion mechanism and container name
WalkthroughThe updates focus on enhancing the workflow and environment for bioinformatics processing, emphasizing the transition from Conda to Singularity for better reproducibility and efficiency. Key changes include instructions for file conversion, guidance against Conda for production workflows, detailed steps for using and creating Singularity images, and updates to the workflow logic for streamlined processing. Additionally, fixes address issues with Singularity container startup and profile merging, ensuring smoother operations in LSF cluster environments. Changes
Assessment against linked issues
The assessment indicates that most objectives from the linked issues have been addressed, with a notable exception regarding the specific configuration order issue for profile merging. This might require further clarification or direct inspection of the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (4)
- README.md (4 hunks)
- main.nf (6 hunks)
- nextflow.config (1 hunks)
- test/test1.sh (1 hunks)
Additional comments: 8
nextflow.config (1)
- 64-72: The changes to the
singularityprofile in thenextflow.configfile are well-implemented and align with the PR objectives of enhancing reproducibility and environment encapsulation. The configuration now automatically pulls the Docker image and stores it in the cache directory, which is a significant improvement for workflow execution. Additionally, the use of--no-homeinsingularity.runOptionsis a good practice to avoid potential issues with environment setup from the user's home directory, thus ensuring a more reproducible workflow environment.test/test1.sh (1)
- 13-13: Changing the default value of
environmentProfilefrom "conda" to "singularity" intest/test1.shis consistent with the PR objectives and the broader shift towards using Singularity for environment management. This change ensures that tests run in an environment that matches the updated workflow recommendations, which is crucial for maintaining consistency and reliability in testing.main.nf (4)
- 179-182: The modification to the
bamToFastqprocess to directly output tuples is a significant improvement in terms of workflow clarity and efficiency. This change simplifies the data flow between processes and reduces the complexity of channel operations, aligning with the PR objectives of enhancing the project's structure and maintainability.- 211-214: The changes to the
nameSortUnpairedFastqsprocess to accept tuples directly from thebamToFastqprocess are well-implemented. This adjustment further streamlines the workflow by eliminating unnecessary channel creation steps, contributing to the overall goal of simplifying the workflow logic.- 247-250: Similarly, the modifications to the
nameSortPairedFastqsprocess to directly accept tuples enhance the workflow's efficiency and clarity. By directly handling tuples, the process becomes more straightforward and aligns with the restructuring efforts aimed at improving the workflow's modularity and maintainability.- 272-303: The integration of channel operations for paired and unpaired FASTQ files into the workflow block is a commendable effort towards simplifying the workflow logic. This restructuring makes the handling of different types of FASTQ files more efficient and intuitive, which is crucial for the scalability and maintainability of the project. The use of
flatMapandgroupByoperations to organize FASTQ files before sorting is a good practice that enhances the workflow's logic and readability.README.md (2)
- 48-52: The section on Quickstart with Conda explicitly advises against using Conda for reproducibly running workflows, emphasizing its use primarily for development. This clear guidance helps set expectations for users regarding the recommended environment management tools. However, it's crucial to ensure that this stance is consistent across all documentation and support channels to avoid confusion.
The caution against using Conda for workflow execution is well-placed and contributes to setting clear expectations for users.
- 196-200: The release notes for version 1.3.0 mention the automatic creation of cached Singularity images by Nextflow and the clarification of Conda's role for development purposes. These notes succinctly communicate the changes and their implications for users. It's important to ensure that these release notes are also reflected in the project's changelog or version history documentation for consistency.
The release notes effectively communicate important changes and their implications for users. Ensure consistency with the project's changelog or version history documentation.
Signed-off-by: Philip R. Kensche <p.kensche@dkfz-heidelberg.de>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (120)
test/reference/test1_paired.bam_fastqs/default_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/default_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/default_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/default_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/default_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run1_gerald_D1VCPACXX_1_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run1_gerald_D1VCPACXX_1_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run1_gerald_D1VCPACXX_1_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run1_gerald_D1VCPACXX_1_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run1_gerald_D1VCPACXX_1_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run2_gerald_D1VCPACXX_2_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run2_gerald_D1VCPACXX_2_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run2_gerald_D1VCPACXX_2_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run2_gerald_D1VCPACXX_2_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run2_gerald_D1VCPACXX_2_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run3_gerald_D1VCPACXX_3_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run3_gerald_D1VCPACXX_3_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run3_gerald_D1VCPACXX_3_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run3_gerald_D1VCPACXX_3_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run3_gerald_D1VCPACXX_3_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run4_gerald_D1VCPACXX_4_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run4_gerald_D1VCPACXX_4_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run4_gerald_D1VCPACXX_4_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run4_gerald_D1VCPACXX_4_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run4_gerald_D1VCPACXX_4_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run5_gerald_D1VCPACXX_5_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run5_gerald_D1VCPACXX_5_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run5_gerald_D1VCPACXX_5_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run5_gerald_D1VCPACXX_5_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_fastqs/run5_gerald_D1VCPACXX_5_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/default_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/default_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/default_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/default_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/default_U2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_U2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_U2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_U2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_U2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_paired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_U2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/default_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/default_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/default_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/default_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/default_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run1_gerald_D1VCPACXX_1_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run1_gerald_D1VCPACXX_1_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run1_gerald_D1VCPACXX_1_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run1_gerald_D1VCPACXX_1_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run1_gerald_D1VCPACXX_1_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run2_gerald_D1VCPACXX_2_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run2_gerald_D1VCPACXX_2_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run2_gerald_D1VCPACXX_2_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run2_gerald_D1VCPACXX_2_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run2_gerald_D1VCPACXX_2_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run3_gerald_D1VCPACXX_3_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run3_gerald_D1VCPACXX_3_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run3_gerald_D1VCPACXX_3_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run3_gerald_D1VCPACXX_3_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run3_gerald_D1VCPACXX_3_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run4_gerald_D1VCPACXX_4_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run4_gerald_D1VCPACXX_4_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run4_gerald_D1VCPACXX_4_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run4_gerald_D1VCPACXX_4_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run4_gerald_D1VCPACXX_4_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run5_gerald_D1VCPACXX_5_R1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run5_gerald_D1VCPACXX_5_R2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run5_gerald_D1VCPACXX_5_S.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run5_gerald_D1VCPACXX_5_U1.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_fastqs/run5_gerald_D1VCPACXX_5_U2.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/default_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/default_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/default_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/default_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/default_U2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_U2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_U2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_U2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_U2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_R1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_R2.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_S.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_U1.sorted.fastq.gzis excluded by:!**/*.gztest/reference/test1_unpaired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_U2.sorted.fastq.gzis excluded by:!**/*.gz
Files selected for processing (1)
- test/test1.sh (3 hunks)
Additional comments: 4
test/test1.sh (4)
- 13-13: The change in the default value of
environmentProfilefrom "conda" to "singularity" aligns with the PR's objectives and is correctly implemented.- 81-81: The updated paths for input files, including a reference directory, are a good practice for organizing test files and are correctly implemented.
Also applies to: 101-101
- 88-92: The addition of assertions to compare the number of reads in BAM files and output directories is a robust way to verify the correctness of the conversion process and is correctly implemented.
Also applies to: 108-112
- 123-123: The call to
testFinishedat the end of the script is correctly implemented, providing a clear summary of the test outcomes.
Signed-off-by: Philip R. Kensche <p.kensche@dkfz-heidelberg.de>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- README.md (4 hunks)
Additional comments: 3
README.md (3)
- 48-65: The clarification on the use of Conda primarily for development purposes and the recommendation against using it for running workflows are well articulated. This guidance helps set clear expectations for users regarding the tool's role and encourages best practices for reproducibility. No further refinements are needed here.
- 196-201: The release notes for version 1.3.0 effectively summarize the key changes, including the automation of cached Singularity image creation and the clarification of Conda's role. These notes align well with the updates made in the README.md file and provide users with a clear understanding of the improvements and patches in this version.
- 139-140: The instructions for manually pulling the Singularity container are clear and straightforward, providing users with an alternative method to handle the container. This addition is helpful and complements the earlier segment on running the workflow with Singularity.
Signed-off-by: Philip R. Kensche <p.kensche@dkfz-heidelberg.de>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- README.md (5 hunks)
Additional comments: 3
README.md (3)
- 38-47: The warning about the concurrency issue when downloading cached containers is important for users running multiple workflows simultaneously. However, the response from a previous comment indicates that providing a specific solution to this problem is considered beyond the scope of this project. It's understandable given the complexity of the issue, but acknowledging this limitation in the documentation could help set the right expectations for users.
It's good to see that potential concurrency issues are acknowledged. Users should be aware of this limitation and plan their workflows accordingly.
- 48-52: The note on Conda not being recommended for reproducibly running workflows is a significant shift and aligns with the PR's objectives to promote Singularity for better reproducibility and environment encapsulation. This clarification is helpful for users and developers, setting clear expectations about the supported and recommended tools for running the workflow.
The clarification on the use of Conda primarily for development purposes and not for running workflows is well-placed and aligns with the project's goals for reproducibility.
- 115-257: The example provided to illustrate the output directory structure and file naming convention is very informative and helps users understand what to expect from the workflow's output. This addition enhances the documentation's clarity and usability, especially for users unfamiliar with BAM and FASTQ file formats.
The inclusion of a detailed example illustrating the output directory structure and file naming convention is a valuable addition to the documentation, enhancing clarity and usability.
… with the old "lsf" profile, even if additionally the "singularity" profile was used. Signed-off-by: Philip R. Kensche <p.kensche@dkfz-heidelberg.de>
…s from different profiles. Signed-off-by: Philip R. Kensche <p.kensche@dkfz-heidelberg.de>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
test-environment.ymlis excluded by:!**/*.yml
Files selected for processing (1)
- nextflow.config (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- nextflow.config
|
The current settings seem to be correct, but Nextflow does not what I'd expect of it. With the current |
…ion-testsd/run.sh. Signed-off-by: Philip R. Kensche <p.kensche@dkfz-heidelberg.de>
Signed-off-by: Philip R. Kensche <p.kensche@dkfz-heidelberg.de>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (122)
integration-tests/reference/test1_paired.bam_fastqs/default_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/default_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/default_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/default_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/default_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run1_gerald_D1VCPACXX_1_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run1_gerald_D1VCPACXX_1_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run1_gerald_D1VCPACXX_1_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run1_gerald_D1VCPACXX_1_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run1_gerald_D1VCPACXX_1_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run2_gerald_D1VCPACXX_2_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run2_gerald_D1VCPACXX_2_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run2_gerald_D1VCPACXX_2_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run2_gerald_D1VCPACXX_2_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run2_gerald_D1VCPACXX_2_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run3_gerald_D1VCPACXX_3_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run3_gerald_D1VCPACXX_3_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run3_gerald_D1VCPACXX_3_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run3_gerald_D1VCPACXX_3_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run3_gerald_D1VCPACXX_3_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run4_gerald_D1VCPACXX_4_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run4_gerald_D1VCPACXX_4_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run4_gerald_D1VCPACXX_4_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run4_gerald_D1VCPACXX_4_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run4_gerald_D1VCPACXX_4_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run5_gerald_D1VCPACXX_5_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run5_gerald_D1VCPACXX_5_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run5_gerald_D1VCPACXX_5_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run5_gerald_D1VCPACXX_5_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_fastqs/run5_gerald_D1VCPACXX_5_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/default_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/default_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/default_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/default_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/default_U2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_U2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_U2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_U2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_U2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_paired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_U2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/default_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/default_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/default_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/default_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/default_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run1_gerald_D1VCPACXX_1_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run1_gerald_D1VCPACXX_1_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run1_gerald_D1VCPACXX_1_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run1_gerald_D1VCPACXX_1_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run1_gerald_D1VCPACXX_1_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run2_gerald_D1VCPACXX_2_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run2_gerald_D1VCPACXX_2_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run2_gerald_D1VCPACXX_2_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run2_gerald_D1VCPACXX_2_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run2_gerald_D1VCPACXX_2_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run3_gerald_D1VCPACXX_3_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run3_gerald_D1VCPACXX_3_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run3_gerald_D1VCPACXX_3_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run3_gerald_D1VCPACXX_3_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run3_gerald_D1VCPACXX_3_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run4_gerald_D1VCPACXX_4_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run4_gerald_D1VCPACXX_4_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run4_gerald_D1VCPACXX_4_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run4_gerald_D1VCPACXX_4_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run4_gerald_D1VCPACXX_4_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run5_gerald_D1VCPACXX_5_R1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run5_gerald_D1VCPACXX_5_R2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run5_gerald_D1VCPACXX_5_S.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run5_gerald_D1VCPACXX_5_U1.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_fastqs/run5_gerald_D1VCPACXX_5_U2.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/default_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/default_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/default_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/default_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/default_U2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run1_gerald_D1VCPACXX_1_U2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run2_gerald_D1VCPACXX_2_U2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run3_gerald_D1VCPACXX_3_U2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run4_gerald_D1VCPACXX_4_U2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_R1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_R2.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_S.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_U1.sorted.fastq.gzis excluded by:!**/*.gzintegration-tests/reference/test1_unpaired.bam_sorted_fastqs/run5_gerald_D1VCPACXX_5_U2.sorted.fastq.gzis excluded by:!**/*.gztask-environment.ymlis excluded by:!**/*.ymltest-environment.ymlis excluded by:!**/*.yml
Files selected for processing (9)
- .circleci/config.yml (2 hunks)
- .dockerignore (1 hunks)
- .gitignore (1 hunks)
- README.md (5 hunks)
- container-specs/Dockerfile (1 hunks)
- container-specs/Singularity.def (1 hunks)
- integration-tests/run.sh (4 hunks)
- nextflow.config (1 hunks)
- nf-bam2fastq.iml (1 hunks)
Files skipped from review due to trivial changes (1)
- .gitignore
Files skipped from review as they are similar to previous changes (1)
- nextflow.config
Additional comments: 21
.dockerignore (5)
- 12-12: The addition of
.githubto the.dockerignorefile is appropriate, as it ensures that GitHub-specific metadata and workflows are not included in Docker images, which is generally a good practice for keeping images lean.- 14-14: Adding
*Env/to the.dockerignorefile is a good practice, as it likely refers to environment directories that should not be included in Docker images. This helps in avoiding unnecessary files in the build context.- 15-15: Ignoring
integration-tests/in Docker builds is sensible, especially if these tests are not required to run within the Docker container. This exclusion helps in reducing the size of the Docker image.- 16-16: Excluding
.run/directories is generally a good practice, as these directories might contain runtime data or logs that are not necessary for the Docker image.- 17-17: Ignoring
*.logfiles is a good practice to keep Docker images clean from log files that might have been generated during development or testing.container-specs/Singularity.def (1)
- 1-15: The
Singularity.deffile is well-documented and correctly sets up the conversion from a Docker image to a Singularity image. Usingdocker-daemonas the bootstrap method allows for using locally tagged images, which is useful for development and testing. The%environmentsection correctly sources the Conda environment, ensuring that the necessary environment is activated when the Singularity container is executed. This setup is crucial for the workflow's proper execution and maintainability.nf-bam2fastq.iml (2)
- 16-16: Adding an exclusion for
integration-tests/output-allis appropriate, as it prevents the IDE from indexing or otherwise interacting with output files generated by integration tests. This can improve the performance of the IDE and keep the project view clean.- 22-22: The addition of a library entry for
groovyat the project level is a good practice, especially if Groovy is used within the project. This ensures that the IDE correctly recognizes Groovy syntax and can provide relevant code assistance.container-specs/Dockerfile (1)
- 1-36: The
Dockerfileis well-structured and follows best practices for Docker image creation. Starting from a Miniconda base image is appropriate for Python-based workflows. The use of environment variables for proxy configuration is a good practice, especially in environments behind corporate proxies. The environment setup, including the creation of a Conda environment fromtask-environment.yml, ensures that all necessary dependencies are installed and isolated within the container. The entry point configuration usingconda runwith the-poption is a clever workaround for theNoWritableEnvsDirError, ensuring that the Conda environment is correctly activated when the container starts. Overall, this Dockerfile is well-crafted to meet the workflow's requirements..circleci/config.yml (3)
- 20-20: Updating the cache key to include a checksum of
tests-environment.ymlis a good practice, as it ensures that the cache is invalidated and updated whenever the test environment changes. This helps in maintaining the accuracy and efficiency of CI builds.- 26-26: Changing the test script path to
integration-tests/run.shand specifying the output directory and nextflow environment path is appropriate. This ensures that the integration tests are executed correctly and that the results are stored in the specified directory. It's important for maintaining the accuracy of CI test results.- 41-41: The job name
integration-testsin the workflows section correctly reflects the purpose of the job, which is to run integration tests. This naming convention is clear and helps in understanding the CI configuration.integration-tests/run.sh (2)
- 14-17: Setting the default environment profile to
singularityand specifying the output directory and nextflow environment path are appropriate changes. These adjustments ensure that the integration tests are executed in the intended environment and that the results are stored correctly. This is important for the accuracy and reproducibility of the tests.- 101-127: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [76-126]
The addition of the MD5 checksum comparison loop is a significant improvement to the integration tests. This loop ensures that the output files match the expected reference files, providing a robust method for verifying the correctness of the workflow's output. This addition enhances the reliability and accuracy of the integration tests.
README.md (7)
- 7-19: The Quickstart with Docker section provides clear instructions for running the workflow using Docker. However, it's important to ensure that the Docker images referenced are kept up-to-date and that users are directed to check for the latest versions. Additionally, consider explicitly mentioning the need for Docker to be installed and running on the user's machine as a prerequisite.
- Ensure Docker images are up-to-date and mention Docker installation as a prerequisite.
- 22-47: The Quickstart with Singularity section effectively introduces Singularity as an alternative to Docker for cluster environments. The warning about potential concurrency issues when downloading cached containers is particularly useful. To enhance this section:
Consider adding a brief explanation or link on how to install Singularity for users who might be unfamiliar with it.
Clarify the
$repoDirvariable in the code snippet for users who might not understand what it represents.Add Singularity installation guidance and clarify
$repoDir.
- 48-62: The Quickstart with Conda section clearly advises against using Conda for running workflows due to reproducibility concerns, which is a good practice. However, since Conda is mentioned as an option for development:
It might be helpful to provide more context on why Conda is suitable for development but not for running workflows, to help users make informed decisions.
Ensure that the provided Conda environment file (
task-environment.yml) is up-to-date and includes all necessary dependencies for development purposes.Provide more context on Conda's suitability for development and ensure the Conda environment file is up-to-date.
- 4-69: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [68-73]
The Remarks section offers valuable insights into the workflow's behavior and considerations. To further improve this section:
Consider providing links to external tools mentioned (e.g., biobambam2, UNIX coreutils) for users who may want to explore these tools further.
Clarify the implications of sorting FASTQ files by their IDs and how it affects subsequent processing steps, possibly with a brief example or scenario.
Provide links to external tools and clarify the implications of sorting FASTQ files.
- 280-290: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [258-287]
The Environment and Execution section provides a comprehensive overview of Nextflow's
-profileparameter and the predefined profiles. To enhance this section:
Clarify the distinction between "environment" and "executor" profiles for users who may not be familiar with these concepts.
Include an example of how to change the default storage locations for Conda environments and Singularity containers, as this could be a common task for users managing their workflow installations.
Clarify profile distinctions and include an example of changing default storage locations.
- 313-355: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [299-336]
The Development section provides clear instructions for running integration tests and the container release process. To further refine this section:
Clarify the
$profilevariable in the integration test command to help users understand how to specify the environment for the tests.For the container release process, consider adding a note about ensuring that the user has the necessary permissions to push containers to the Github Container Registry.
Clarify the
$profilevariable in integration tests and add a note on permissions for container release.
- 341-351: The Release Notes section is well-organized and provides a clear changelog for the project. To improve this section:
Ensure consistency in the presentation of version numbers and release dates. For example, avoid using commas in dates (e.g., "March, 2024" should be "March 2024").
Consider linking to relevant issues or pull requests for each listed change to provide users with more context and the ability to explore specific updates in detail.
Ensure consistency in date formatting and consider linking to relevant issues or pull requests.
| integration-tests/ | ||
| .run/ | ||
| *.log | ||
| *Env/ |
There was a problem hiding this comment.
The addition of *Env/ again at the end of the .dockerignore file seems to be a duplicate entry. This redundancy does not impact the functionality but could be cleaned up for clarity and maintainability.
- *Env/Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| *Env/ |
vinjana
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
singularity exec, while 23 usessingularity run, which impacts process isolation. This also Closes Fix Singularity container startup #52.Singularity.def, in case the automatic conversion by Nextflow does not work.README.md. Otherwise, it should not be used.test/test1.shtointegration-tests/run.sh. Changed order of parameters.Summary by CodeRabbit
main.nffor streamlined channel operations with paired and unpaired fastqs.test/test1.shto use "singularity" and optimized file paths.nextflow.configfor better reproducibility..circleci/config.ymlfor integration tests..dockerignoreand.gitignorefor Docker image building.Singularity.deffile for converting Docker images to Singularity images.