Input csv bug fixes regarding sample id #173

TomasCumlin · 2023-09-05T11:48:39Z

Description

Summary of the changes made:

get_meta.nf modifed
Exit workflow if ID duplicates, non-ASCII characters, incorrect length of ID or incorrect platform added
8 additional files with local changes that are not to be merged with the master repository.

Primary function of PR

Hotfix
Minor functionality improvement
Major functionality improvement / New type of analysis
Backward-breaking functionality

Testing

Either describe a procedure, or add data; that confirms that the PR resolves what it sets out to do

Sign-offs

Code reviewed by @octocat
Code tested by @octocat

Pulling changes from mster#

sylvinite

Nice method module updates! Very pro!

Some comments about things that accidently slipped in.

Also, to be sure, the reference updates you do for the submodules are more recent versions, correct?

sylvinite · 2023-09-05T11:52:31Z

main.nf


 include { CALL_STAPHYLOCOCCUS_AUREUS        } from './workflows/staphylococcus_aureus.nf'
 include { CALL_MYCOBACTERIUM_TUBERCULOSIS   } from './workflows/mycobacterium_tuberculosis.nf'

+<<<<<<< HEAD


Contains bad merge

Yeah this happens when you merge and have conflicts that you need to resolve. These lines should be deleted.

sylvinite · 2023-09-05T11:53:20Z

configs/nextflow.base.config

@@ -1,22 +1,22 @@
 params {


Please let the example config remain as is. Make a seperate config for your local system, or add it to .gitignore

sylvinite · 2023-09-05T11:53:25Z

assets/test_data/samplelist.csv

@@ -1,2 +1,3 @@
 id,platform,read1,read2
-p1000,illumina,/path/to/JASEN/assets/test_data/sequencing_data/saureus_100k/saureus_large_R1_001.fastq.gz,/path/to/JASEN/assets/test_data/sequencing_data/saureus_100k/saureus_large_R2_001.fastq.gz


Please let the example config remain as is. Make a seperate config for your local system, or add it to .gitignore

ryanjameskennedy · 2023-09-05T13:42:37Z

main.nf

+
+
+// Function for platform and paired-end or single-end
+def get_meta(LinkedHashMap row) {


This has been moved to methods/get_meta.nf. Remove/copy changes there:)

ryanjameskennedy · 2023-09-05T13:44:00Z

main.nf

 workflow {
    if (workflow.profile == "staphylococcus_aureus") {
        CALL_STAPHYLOCOCCUS_AUREUS()
    } else if (workflow.profile == "mycobacterium_tuberculosis") {
        CALL_MYCOBACTERIUM_TUBERCULOSIS()
    }
+>>>>>>> master


Remove this line (related to conflicts from merge)

ryanjameskennedy · 2023-09-05T13:45:16Z

main.nf

+  emit: 
+    pipeline_result = create_analysis_result.output
+    cdm_import = export_to_cdm.output
+=======


Everything above and including this line should be removed as it is in the workflows.

ryanjameskennedy · 2023-09-05T13:46:21Z

methods/get_meta.nf

@@ -1,14 +1,45 @@
+idList = []     // Global variable to save IDs from samplesheet


Can you not create a list within the get_meta.nf method instead?

ryanjameskennedy · 2023-09-05T13:46:54Z

main.nf

@@ -1,14 +1,179 @@
 #!/usr/bin/env nextflow

 nextflow.enable.dsl=2
+idList = []     // Global variable to save IDs from samplesheet


This should be removed

ryanjameskennedy · 2023-09-05T13:50:03Z

nextflow-modules/modules/chewbbaca/main.nf

@@ -59,14 +59,22 @@ process chewbbaca_create_batch_list {
  script:
    output = "batch_input.list"
    """
-    realpath $maskedAssembly > $output
+    touch /projects/mikro/nobackup/gms/jasen2/chewbbaca_temp.txt


All of the changes in this file need to be reverted back to the original

ryanjameskennedy · 2023-09-05T13:54:40Z

methods/get_meta.nf

+  platforms = ["illumina", "nanopore", "pacbio", "iontorrent"]
+
+  // Error messages
+  idErrorMessage = "\nERROR: Please check input samplesheet -> ID '${row.id}' is not a unique name in the samplesheet."


I think this is a good effort but I reckon that we should maybe consider making a new method called check_input.nf because this check plays no role in the act of getting the metadata. Revert this method to it's original format and create a method file.

ryanjameskennedy · 2023-09-05T13:57:02Z

assets/resfinder_db

Not sure why these databases have two changes from the merge. Could it be the newline formatting on different platforms? @sylvinite @LordRust

Doublechecked and its cus the database "updated to" is actually 3 months older.
So the commits to change the submodule commitid target need to be removed.

ryanjameskennedy

In general I would revert all 9 of these files back to their original form and just create a new method (check_input.nf). Some advice that will save you some pain:

Don't use git add . -> Rather use git add <filepath> to be more specific with what you wish to push.
Use git status to see which files are staged to be commited before you git commit anything.

I'm happy to see you giving this a go! You will pick everything up super quickly! Keep up the good work!

Tomas Cumlin and others added 7 commits June 12, 2023 16:29

testing branches

3f21966

temporary modification of chew main.nf

fe6e97b

ID check added to main.nf

5ef02ea

merged changes from master

a59a8b6

pulled changes from master

ba6693e

Merge branch 'master' into issue59_backup

dccf9df

Pulling changes from mster#

issue 59 and 60

fd2969b

TomasCumlin requested a review from ryanjameskennedy September 5, 2023 11:48

TomasCumlin requested a review from sylvinite as a code owner September 5, 2023 11:48

sylvinite requested changes Sep 5, 2023

View reviewed changes

This was linked to issues Sep 5, 2023

JASEN crashes when there are multiple files with identical sample-id in input csv #59

Open

Enforce limitation on characters in sample ids? #60

Open

ryanjameskennedy changed the title ~~Issue 59 & 60~~ Input csv bug fixes regarding sample id Sep 5, 2023

ryanjameskennedy reviewed Sep 5, 2023

View reviewed changes

ryanjameskennedy requested changes Sep 5, 2023

View reviewed changes

ryanjameskennedy closed this Jan 10, 2024

This was unlinked from issues Feb 14, 2024

Enforce limitation on characters in sample ids? #60

Open

JASEN crashes when there are multiple files with identical sample-id in input csv #59

Open

ryanjameskennedy deleted the issue59_backup branch June 3, 2024 08:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Input csv bug fixes regarding sample id #173

Input csv bug fixes regarding sample id #173

TomasCumlin commented Sep 5, 2023 •

edited by sylvinite

Loading

sylvinite left a comment

sylvinite Sep 5, 2023

ryanjameskennedy Sep 5, 2023

sylvinite Sep 5, 2023

sylvinite Sep 5, 2023

ryanjameskennedy Sep 5, 2023

ryanjameskennedy Sep 5, 2023

ryanjameskennedy Sep 5, 2023 •

edited

Loading

ryanjameskennedy Sep 5, 2023

ryanjameskennedy Sep 5, 2023

ryanjameskennedy Sep 5, 2023

ryanjameskennedy Sep 5, 2023

ryanjameskennedy Sep 5, 2023

sylvinite Sep 5, 2023

ryanjameskennedy left a comment •

edited

Loading

		@@ -1,2 +1,3 @@
		id,platform,read1,read2
		p1000,illumina,/path/to/JASEN/assets/test_data/sequencing_data/saureus_100k/saureus_large_R1_001.fastq.gz,/path/to/JASEN/assets/test_data/sequencing_data/saureus_100k/saureus_large_R2_001.fastq.gz



		// Function for platform and paired-end or single-end
		def get_meta(LinkedHashMap row) {

		@@ -1,14 +1,45 @@
		idList = [] // Global variable to save IDs from samplesheet

Input csv bug fixes regarding sample id #173

Input csv bug fixes regarding sample id #173

Conversation

TomasCumlin commented Sep 5, 2023 • edited by sylvinite Loading

Description

Primary function of PR

Testing

Sign-offs

sylvinite left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanjameskennedy Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanjameskennedy left a comment • edited Loading

Choose a reason for hiding this comment

TomasCumlin commented Sep 5, 2023 •

edited by sylvinite

Loading

ryanjameskennedy Sep 5, 2023 •

edited

Loading

ryanjameskennedy left a comment •

edited

Loading