Allowed for lane-level review autoqc checks.

wtsi-npg · Oct 25, 2024 · ea3a4d0 · ea3a4d0
1 parent 83d41ad
commit ea3a4d0
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 11 deletions.
diff --git a/Changes b/Changes
@@ -1,6 +1,11 @@
 LIST OF CHANGES
 ---------------
 
+ - npg_pipeline::function::autoqc
+   - Simplified the flow of the code.
+   - Made clearer logic for choosing QC checks to run, provided comments.
+   - Allowed for running the review QC check on lanes.
+
 release 68.6.0 (2024-10-24)
  - The runfolder_path attribute is passed to the constructor of the review
    autoqc check object when deciding whether this check should run.

diff --git a/MANIFEST b/MANIFEST
@@ -1042,6 +1042,7 @@ t/data/release/config/notify_off/product_release.yml
 t/data/release/config/notify_on/product_release.yml
 t/data/release/config/qc_review/product_release.yml
 t/data/release/config/qc_review_default/product_release.yml
+t/data/release/config/qc_review_default_with_lane/product_release.yml
 t/data/release/config/pp_archival/product_release.yml
 t/data/release/config/pp_archival/product_release_no_array.yml
 t/data/release/config/pp_archival/product_release_no_filters.yml

diff --git a/lib/npg_pipeline/function/autoqc.pm b/lib/npg_pipeline/function/autoqc.pm
@@ -19,24 +19,22 @@ with q{npg_pipeline::function::util};
 
 our $VERSION = '0';
 
-Readonly::Scalar my $QC_SCRIPT_NAME           => q{qc};
+Readonly::Scalar my $QC_SCRIPT_NAME => q{qc};
+
 Readonly::Array  my @CHECKS_NEED_PAIREDNESS_INFO => qw/
                                                  insert_size
                                                  gc_fraction
                                                  qX_yield
                                                       /;
-##no critic (RegularExpressions::RequireBracesForMultiline)
-##no critic (RegularExpressions::ProhibitComplexRegexes)
-Readonly::Scalar my $TARGET_FILE_CHECK_RE => qr/\A
+
+Readonly::Scalar my $TARGET_FILE_CHECK_RE => qr{ \A
                                               adapter |
                                              bcfstats |
                                         verify_bam_id |
                                              genotype |
                                      pulldown_metrics |
-                                     haplotag_metrics |
-                                               review
-                                               \Z/xms;
-## use critic
+                                     haplotag_metrics
+                                                 \Z }xms;
 
 has q{qc_to_run}       => (isa      => q{Str},
                            is       => q{ro},
@@ -257,6 +255,12 @@ sub _should_run {
     # For indexed runs, run on individual lanes if they are pools of samples.
     return $self->is_indexed() && $is_lane && $is_pool ? 1 : 0;
   }
+  if ($self->qc_to_run() eq 'review') {
+    # This check should never run on tag zero, other entities are fair game.
+    if ($is_plex && $product->is_tag_zero_product) {
+      return 0;
+    }
+  }
   if ($self->qc_to_run() =~ $TARGET_FILE_CHECK_RE) {
     # Do not run on tag zero files, unmerged or merged.
     # Do not run on lane-level data if the lane is a pool.

diff --git a/t/20-function-autoqc.t b/t/20-function-autoqc.t
@@ -576,7 +576,7 @@ subtest 'genotype and gc_fraction and bcfstats' => sub {
 };
 
 subtest 'review' => sub {
-  plan tests => 8;
+  plan tests => 10;
 
   local $ENV{NPG_CACHED_SAMPLESHEET_FILE} = 't/data/samplesheet_8747.csv';
 
@@ -640,9 +640,26 @@ subtest 'review' => sub {
   $da = $qc->create();
   ok ($da && (@{$da} == 6), '6 definitions returned');
   %definitions = map { $_->composition->freeze2rpt => $_ } @{$da};
-  @expected_rpt_lists = map { "8747:$_:168"} qw/1 2 3 4 5 6/;
+  @expected_rpt_lists = map { "8747:$_:168"} (1 .. 6);
   is_deeply ([sort keys %definitions], \@expected_rpt_lists,
     'definitions are for correct entities');
+
+  $qc = npg_pipeline::function::autoqc->new(
+    qc_to_run         => 'review',
+    is_indexed        => 1,
+    id_run            => 8747,
+    runfolder_path    => $rf_path,
+    timestamp         => q{today},
+    conf_path         => q{t/data/release/config/qc_review_default_with_lane},
+    resource          => $default
+  );
+
+  $da = $qc->create();
+  ok ($da && (@{$da} == 14), '14 definitions returned');
+  %definitions = map { $_->composition->freeze2rpt => $_ } @{$da};
+  push @expected_rpt_lists, map { "8747:$_" } (1 .. 8); 
+  is_deeply ([sort keys %definitions], [sort @expected_rpt_lists],
+    'definitions for lanes are added');
 };
 
 subtest 'interop' => sub {

diff --git a/t/data/release/config/qc_review_default/product_release.yml b/t/data/release/config/qc_review_default/product_release.yml
@@ -11,7 +11,6 @@ default:
   gbs_pipeline: &gbs_pipeline_accessible
     allowed: true
   robo_qc:
-    qc_type: "mqc"
     criteria:
       - applicability_criteria:
           lims:

diff --git a/t/data/release/config/qc_review_default_with_lane/product_release.yml b/t/data/release/config/qc_review_default_with_lane/product_release.yml
@@ -0,0 +1,29 @@
+---
+common:
+  irods: &no_irods
+    enable: false
+default:
+  irods: &default_irods
+    enable: true
+  data_deletion:
+    staging_deletion_delay: 30
+  markdup_method: "biobambam"
+  gbs_pipeline: &gbs_pipeline_accessible
+    allowed: true
+  robo_qc:
+    criteria:
+      - applicability_criteria:
+          lims:
+            is_control: 1
+          sequencing_run:
+            platform_HiSeq: 1
+        acceptance_criteria:
+          - "sequence_error.pass"
+      - applicability_criteria:
+          lims:
+            is_lane: 1
+          sequencing_run:
+            platform_HiSeq: 1
+        acceptance_criteria:
+          - "sequence_error.pass"
+