Skip to content

Commit

Permalink
Merge pull request wtsi-npg#888 from wtsi-npg/devel
Browse files Browse the repository at this point in the history 
pull from devel to master to create release 73.0.0
  • Loading branch information
@jmtcsngr
jmtcsngr authored Oct 24, 2024
2 parents 91c876d + 663746e commit a9790da
Show file tree
Hide file tree
Showing 24 changed files with 127 additions and 218 deletions.
15 changes: 15 additions & 0 deletions Changes
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
LIST OF CHANGES FOR NPG-QC PACKAGE

release 73.0.0 (2024-10-24)
- Removed from the repository unused test data.
- npg_qc::autoqc::check::review:
1. Previously the code allowed for an empty applicability_criteria hash,
which resulted in a particular set of QC criteria being applied to every
and any product. Very early on this was an intended behaviour for UKB
data. The main filter was the study id. The default section of the product
configuration file does not have an external filter, so there is a real
danger of the review check being run indiscriminately for any product.
While this will never be an intention, small errors in the YML file might
have this effect.
2. Ability to set UQC outcomes was introduced for the Heron project. This
functionality was never used and is unlikely to be used in future, it is
now removed.

release 72.2.1 (2024-10-04)
- Added .github/dependabot.yml file to auto-update GitHub actions
- GitHub CI - updated deprecated v2 runner action to v3
Expand Down
19 changes: 2 additions & 17 deletions MANIFEST
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,8 +520,9 @@ t/data/autoqc/review/with_na_criteria/product_release.yml
t/data/autoqc/review/no_criteria_section/product_release.yml
t/data/autoqc/review/not_hash/product_release.yml
t/data/autoqc/review/mqc_type/product_release.yml
t/data/autoqc/review/lims_applicability_empty/product_release.yml
t/data/autoqc/review/no_known_applicability_type/product_release.yml
t/data/autoqc/review/unknown_qc_type/product_release.yml
t/data/autoqc/review/uqc_type/product_release.yml
t/data/autoqc/review/default_and_study_section/product_release.yml
t/data/autoqc/review/default_section/product_release.yml
t/data/autoqc/review/wrong_default_and_study_section/product_release.yml
Expand Down Expand Up @@ -606,22 +607,6 @@ t/data/fixtures/000-mqc_outcome_dict.yml
t/data/fixtures/000-mqc_library_outcome_dict.yml
t/data/fixtures/000-uqc_outcome_dict.yml
t/data/fixtures/autoqc_json.tar.gz
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_1.insert_size.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_1.qX_yield.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_2.insert_size.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_2.qX_yield.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_3.insert_size.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_3.qX_yield.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_4.insert_size.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_4.qX_yield.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_5.insert_size.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_5.qX_yield.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_6.insert_size.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_6.qX_yield.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_7.insert_size.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_7.qX_yield.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_8.insert_size.json
t/data/nfs/sf44/IL2/analysis/123456_IL2_1234/Data/Intensities/Bustard_RTA/PB_cal/archive/qc/1234_8.qX_yield.json
t/data/qcoutcomes/fixtures/000-mqc_library_outcome_dict.yml
t/data/qcoutcomes/fixtures/000-mqc_outcome_dict.yml
t/data/qcoutcomes/fixtures/000-uqc_outcome_dict.yml
Expand Down
74 changes: 38 additions & 36 deletions lib/npg_qc/autoqc/checks/review.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,13 @@ Readonly::Scalar my $QC_TYPE_KEY => q[qc_type];
Readonly::Scalar my $APPLICABILITY_CRITERIA_KEY => q[applicability_criteria];
Readonly::Scalar my $LIMS_APPLICABILITY_CRITERIA_KEY => q[lims];
Readonly::Scalar my $SEQ_APPLICABILITY_CRITERIA_KEY => q[sequencing_run];
Readonly::Array my @APPLICABILITY_CRITERIA_TYPES => (
$LIMS_APPLICABILITY_CRITERIA_KEY, $SEQ_APPLICABILITY_CRITERIA_KEY
);
Readonly::Scalar my $ACCEPTANCE_CRITERIA_KEY => q[acceptance_criteria];

Readonly::Scalar my $QC_TYPE_DEFAULT => q[mqc];
Readonly::Array my @VALID_QC_TYPES => ($QC_TYPE_DEFAULT, q[uqc]);
Readonly::Array my @VALID_QC_TYPES => ($QC_TYPE_DEFAULT);

Readonly::Scalar my $TIMESTAMP_FORMAT_WOFFSET => q[%Y-%m-%dT%T%z];

Expand Down Expand Up @@ -103,11 +106,6 @@ marked as 'Preliminary' (examples: 'Accepted Final',
'Rejected Preliminary'). By default the final_qc_outcome flag is
false and the produced outcomes are preliminary.
A valid User QC outcome is one of the values from the
uqc_outcome_dict table of the npg_qc database. A concept of
the finality and, hence, immutability of the outcome is not
applicable to user QC outcome.
The type of QC outcome can be configured within the Robo QC
section of product configuration. The default type is library
Manual QC.
Expand Down Expand Up @@ -329,8 +327,9 @@ sub execute {
}

$self->result->criteria($self->_criteria);
my $md5 = $self->result->generate_checksum4data($self->result->criteria);
$self->result->criteria_md5($md5);
$self->result->criteria_md5(
$self->result->generate_checksum4data($self->result->criteria)
);
my $err;

try {
Expand All @@ -341,7 +340,7 @@ sub execute {
$self->result->add_comment($err);
};
not $err and $self->result->qc_outcome(
$self->generate_qc_outcome($self->_outcome_type(), $md5));
$self->generate_qc_outcome($self->_outcome_type()));

return;
}
Expand All @@ -368,34 +367,26 @@ sub evaluate {
Returns a hash reference representing the QC outcome.
my $u_outcome = $r->generate_qc_outcome('uqc', $md5);
my $m_outcome = $r->generate_qc_outcome('mqc');
=cut

sub generate_qc_outcome {
my ($self, $outcome_type, $md5) = @_;
my ($self, $outcome_type) = @_;

$outcome_type or croak 'outcome type should be defined';

my $package_name = 'npg_qc::Schema::Mqc::OutcomeDict';
my $pass = $self->result->pass;
#####
# Any of Accepted, Rejected, Undecided outcomes can be returned here
my $outcome = ($outcome_type eq $QC_TYPE_DEFAULT)
? $package_name->generate_short_description(
$self->final_qc_outcome ? 1 : 0, $pass)
: $package_name->generate_short_description_prefix($pass);
my $outcome = $package_name->generate_short_description(
$self->final_qc_outcome ? 1 : 0, $pass);

$outcome_type .= '_outcome';
my $outcome_info = { $outcome_type => $outcome,
timestamp => create_current_timestamp(),
username => $ROBO_KEY};
if ($outcome_type =~ /\Auqc/xms) {
my @r = ($ROBO_KEY, $VERSION);
$md5 and push @r, $md5;
$outcome_info->{'rationale'} = join q[ ], @r;
}

return $outcome_info;
}
Expand Down Expand Up @@ -514,31 +505,42 @@ has '_applicable_criteria' => (
sub _build__applicable_criteria {
my $self = shift;

my $criteria_objs = $self->_robo_config->{$CRITERIA_KEY};
my @applicable = ();
foreach my $co ( @{$criteria_objs} ) {
foreach my $criteria_definition ( @{$self->_robo_config->{$CRITERIA_KEY}} ) {

my $applicability_definition = $criteria_definition->{$APPLICABILITY_CRITERIA_KEY};
$applicability_definition or croak
"$APPLICABILITY_CRITERIA_KEY is not defined for one of RoboQC criteria";

my $c_applicable = 1;
for my $c_type ($LIMS_APPLICABILITY_CRITERIA_KEY, $SEQ_APPLICABILITY_CRITERIA_KEY) {
my $c = $co->{$APPLICABILITY_CRITERIA_KEY}->{$c_type};
if ($c && !$self->_applicability($c, $c_type)) {
$c_applicable = 0;
last;
}
my $one_found = 0;
for my $c_type (@APPLICABILITY_CRITERIA_TYPES) {
exists $applicability_definition->{$c_type} or next;
$one_found = 1;
my $ac = $applicability_definition->{$c_type};
(defined $ac and keys %{$ac}) or croak
"$c_type type applicability criteria is not defined";
$c_applicable = $self->_is_applicable($c_type, $ac);
!$c_applicable && last; # Stop on the first non applicable.
}
$c_applicable or next;
push @applicable, $co;
$one_found or croak 'None of known applicability type criteria is defined. ' .
'Known types: ' . join q[, ], @APPLICABILITY_CRITERIA_TYPES;
$c_applicable && push @applicable, $criteria_definition; # Save if fully applicable.
}

return \@applicable;
}

sub _applicability {
my ($self, $acriteria, $criteria_type) = @_;
sub _is_applicable {
my ($self, $criteria_type, $acriteria) = @_;

($acriteria && $criteria_type) or croak
'The criterium and its type type should be defined';
$criteria_type or croak
'Applicability criteria type is not defined';
$acriteria or croak
"$criteria_type applicability criteria is not defined";
(ref $acriteria eq 'HASH') or croak sprintf
'%s section should be a hash in a robo config for %', $criteria_type, $self->_entity_desc;
'%s section should be a hash in a robo config for %',
$criteria_type, $self->_entity_desc;

my $test = {};
foreach my $prop ( keys %{$acriteria} ) {
Expand Down
124 changes: 3 additions & 121 deletions t/50-schema-result-Review.t
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use strict;
use warnings;
use Test::More tests => 6;
use Test::More tests => 5;
use Test::Exception;
use Moose::Meta::Class;
use Digest::MD5 qw/md5_hex/;
Expand Down Expand Up @@ -306,135 +306,17 @@ subtest 'a full insert/update record with mqc outcome' => sub {
is ($rs->next->description, 'Accepted final', 'outcome has not changed');
};

subtest 'a full insert/update record with uqc outcome' => sub {
plan tests => 36;

my $initial_num_records = $schema->resultset($table)->search({})->count();

my $uqc_table = 'UqcOutcomeEnt';
my $id_seq_composition = t::autoqc_util::find_or_save_composition(
$schema, {'id_run' => 1111,
'position' => 1,
'tag_index' => 33});

my $uqc_rs = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition});
is ($uqc_rs->count, 0, 'no uqc records for this entity');
my $rs = $schema->resultset($table)->search({id_seq_composition => $id_seq_composition});
is ($rs->count, 0, 'no review records for this entity');

my $qc_outcome = {"uqc_outcome" => "Rejected",
"timestamp" => "2018-06-03T12:53:46+0000",
"username" => "robo_qc",
"rationale" => "testrobo"};
my $values = {
id_seq_composition => $id_seq_composition,
library_type => 'common_type',
evaluation_results => {"e1"=>1,"e2"=>0},
criteria => {"and"=>["e1","e2"]},
qc_outcome => $qc_outcome,
pass => 0,
path => 't/data'
};
my $cmd5 = md5_hex(JSON::XS->new()->canonical(1)->encode($values->{criteria}));

isa_ok($schema->resultset($table)->create($values), 'npg_qc::Schema::Result::' . $table);
is ($schema->resultset($table)->search({})->count(), $initial_num_records + 1,
'total number of records in teh review table went up by one');
$rs = $schema->resultset($table)->search({id_seq_composition => $id_seq_composition});
is ($rs->count, 1, 'one row created in the review table');
my $row = $rs->next;
is_deeply ($row->qc_outcome, $qc_outcome, 'qc outcome saved');
is_deeply ($row->evaluation_results, {"e1"=>1,"e2"=>0}, 'evaluation results saved');
is_deeply ($row->criteria, {"and"=>["e1","e2"]}, 'criteria saved');
is ($row->criteria_md5, $cmd5, 'checksum saved');
$uqc_rs = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition});
is ($uqc_rs->count, 1, 'one row created in the uqc table');
my $outcome = $uqc_rs->next;
is ($outcome->rationale, 'testrobo', 'rationale recorded');
is ($outcome->description, 'Rejected', 'correct uqc outcome');
is ($outcome->modified_by, $ENV{USER}, 'correct user');
is ($outcome->username, 'robo_qc', 'correct user');
my $dt = $outcome->last_modified();
is ($dt->year, 2018, 'correct year');
is ($dt->month, '6', 'correct month');
is ($dt->minute, '53', 'correct minute');
is ($dt->second, '46', 'correct second');

$qc_outcome = {"uqc_outcome" => "Accepted",
"timestamp" => "2018-08-05T12:53:46+0000",
"username" => "robo_qc",
"rationale" => "testrobo1"};
$values = {
id_seq_composition => $id_seq_composition,
library_type => 'common_type',
evaluation_results => {"e1"=>1,"e2"=>1},
criteria => {"and"=>["e1","e2"]},
qc_outcome => $qc_outcome,
};
$row->update($values);
is_deeply ($row->evaluation_results, {"e1"=>1,"e2"=>1}, 'evaluation results updated');
is_deeply ($row->qc_outcome, $qc_outcome, 'qc outcome updated');
$outcome = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition})
->next;
is ($outcome->rationale, 'testrobo1', 'rationale updated');
is ($outcome->description, 'Accepted', 'uqc outcome updated');
is ($outcome->last_modified()->month, 8, 'month updated');
is ($outcome->last_modified()->year, 2018, 'correct year');

$id_seq_composition = t::autoqc_util::find_or_save_composition(
$schema, {'id_run' => 1111,
'position' => 1,
'tag_index' => 34});
$uqc_rs = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition});
is ($uqc_rs->count, 0, 'no uqc records for this entity');
my $new_uqc = $schema->resultset($uqc_table)
->new_result({id_seq_composition => $id_seq_composition});
$new_uqc->update_outcome({uqc_outcome => 'Accepted', rationale => 'test1'}, 'user1', 'test');
$uqc_rs = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition});
is ($uqc_rs->count, 1, 'record created');

$values = {
id_seq_composition => $id_seq_composition,
library_type => 'common_type',
evaluation_results => '{"e1":1,"e2":1}',
criteria => '{"and":["e1","e2"]}',
qc_outcome => {"uqc_outcome" => "Undecided",
"timestamp" => "2018-09-03T12:58:43+0000",
"username" => "robo_qc",
"rationale" => "testrobo2"},
pass => undef
};
my $created=$schema->resultset($table)->create($values);

is ($schema->resultset($table)->search({})->count(), $initial_num_records + 2,
'total number of records in teh review table went up by one');
$uqc_rs = $schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition});
is ($uqc_rs->count, 1, 'one uqc record for the entity');
$outcome = $uqc_rs->next;
is ($outcome->rationale, 'testrobo2', 'rationale updated');
is ($outcome->description, 'Undecided', 'outcome updated');
is ($outcome->modified_by, $ENV{USER}, 'correct user');
is ($outcome->username, 'robo_qc', 'correct user');
$dt = $outcome->last_modified();
is ($dt->year, 2018, 'correct year');
is ($dt->month, '9', 'correct month');
is ($dt->minute, '58', 'correct minute');
is ($dt->second, '43', 'correct second');
};


subtest 'unknown outcome type should give an error' => sub {
plan tests => 5;

my $uqc_table = 'UqcOutcomeEnt';
my $id_seq_composition = t::autoqc_util::find_or_save_composition(
$schema, {'id_run' => 1111,
'position' => 1,
'tag_index' => 43});

my $no_records = sub {
is ($schema->resultset($uqc_table)->search({id_seq_composition => $id_seq_composition})->count,
0, 'no uqc records for this entity');
is ($schema->resultset($mqc_table)->search({id_seq_composition => $id_seq_composition})->count,
0, 'no mqc records for this entity');
is ($schema->resultset($table)->search({id_seq_composition => $id_seq_composition})->count,
0, 'no review records for this entity');
};
Expand Down
Loading

0 comments on commit a9790da

Please sign in to comment.