From ea3a4d0c29060a66e00cb09972dd178e8b4d2938 Mon Sep 17 00:00:00 2001 From: Marina Gourtovaia Date: Fri, 25 Oct 2024 17:46:33 +0100 Subject: [PATCH] Allowed for lane-level review autoqc checks. --- Changes | 5 ++++ MANIFEST | 1 + lib/npg_pipeline/function/autoqc.pm | 20 ++++++++----- t/20-function-autoqc.t | 21 ++++++++++++-- .../qc_review_default/product_release.yml | 1 - .../product_release.yml | 29 +++++++++++++++++++ 6 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 t/data/release/config/qc_review_default_with_lane/product_release.yml diff --git a/Changes b/Changes index f51b8497..cda9f141 100644 --- 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 index 0cb54a10..19c74e46 100644 --- 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 index 45ad1b5c..6e70904f 100644 --- 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 index 0fbdac27..4deb5e0a 100644 --- 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 index 00396359..7c0049d8 100644 --- 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 new file mode 100644 index 00000000..0b456e06 --- /dev/null +++ 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" +