Skip to content

Commit

Permalink
Made library_type optional in the review check.
Browse files Browse the repository at this point in the history
  • Loading branch information
mgcam committed Sep 11, 2024
1 parent ca1e7a3 commit cb97e0d
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 42 deletions.
6 changes: 6 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
LIST OF CHANGES FOR NPG-QC PACKAGE

- npg_qc::autoqc::check::review:
The forthcoming lane-level evaluations are impossible if the code continues
to error when library_type attribute is not defined for the entity under
consideration. The library_type attribute is now set when possible, no error
if it is undefined.

release 72.2.0 (2024-08-30)
- npg_qc::autoqc::check::review:
1. To enable access to information about a sequencing run (from RunInfo.xml,
Expand Down
14 changes: 6 additions & 8 deletions lib/npg_qc/Schema/Result/Review.pm
Original file line number Diff line number Diff line change
Expand Up @@ -357,14 +357,12 @@ around [qw/update insert/] => sub {

#####
# Do not accept half-baked results, ie if we have evaluation
# results, we should also have library type and criteria.
# results, we should also have criteria.
if ($data->{'evaluation_results'} and keys %{$data->{'evaluation_results'}}) {
foreach my $name (qw/library_type criteria/) {
my $value = $data->{$name};
my $m = "Evaluation results present, but $name absent";
$value or croak $m;
((not ref $value) or keys %{$value}) or croak $m;
}
my $value = $data->{'criteria'};
my $m = 'Evaluation results present, but criteria absent';
$value or croak $m;
((not ref $value) or keys %{$value}) or croak $m;
}

#####
Expand Down Expand Up @@ -475,7 +473,7 @@ Marina Gourtovaia E<lt>[email protected]<gt>
=head1 LICENSE AND COPYRIGHT
Copyright (C) 2019,2020 Genome Research Ltd.
Copyright (C) 2019,2020, 2924 Genome Research Ltd.
This file is part of NPG.
Expand Down
8 changes: 4 additions & 4 deletions lib/npg_qc/autoqc/checks/review.pm
Original file line number Diff line number Diff line change
Expand Up @@ -568,11 +568,11 @@ has '_criteria' => (
sub _build__criteria {
my $self = shift;

# Save redundant library_type.
# TODO: Save details about applicability instead.
# Library type might be undefined. Example - lane level object.
my $lib_type = $self->lims->library_type;
$lib_type or croak 'Library type is not defined for ' . $self->_entity_desc;
$self->result->library_type($lib_type);
if ($lib_type) {
$self->result->library_type($lib_type);
}

my $num_criteria = scalar @{$self->_applicable_criteria};
if ($num_criteria == 0) {
Expand Down
49 changes: 20 additions & 29 deletions t/50-schema-result-Review.t
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,9 @@ my $schema = Moose::Meta::Class->create_anon_class(
->new_object({})->create_test_db(q[npg_qc::Schema], 't/data/fixtures');

subtest 'reject incomplete results on insert' => sub {
plan tests => 4;
plan tests => 2;

my $values = {
evaluation_results => {"e1"=>1,"e2"=>0},
criteria => {"and"=>["e1","e2"]},
pass => 0,
path => 't/data'
};
throws_ok {$schema->resultset($table)->create($values)}
qr/Evaluation results present, but library_type absent/,
'library type is not defined - error on insert';

$values = {
library_type => 'common_type',
criteria => {},
evaluation_results => {"e1"=>1,"e2"=>0},
Expand All @@ -41,18 +31,6 @@ subtest 'reject incomplete results on insert' => sub {
qr/Evaluation results present, but criteria absent/,
'criteria are not defined - error on insert';

$values = {
evaluation_results => {"e1"=>1,"e2"=>0},
criteria => {},
qc_outcome =>
{"mqc_outcome"=>"Rejected final","timestamp"=>"2018-06-03T12:53:46+0000","username"=>"robo_qc"},
pass => 0,
path => 't/data'
};
throws_ok {$schema->resultset($table)->create($values)}
qr/Evaluation results present, but library_type absent/,
'criteria and library type are not defined - error on insert';

$values = {
library_type => 'common_type',
evaluation_results => {"e1"=>1,"e2"=>0},
Expand All @@ -68,13 +46,27 @@ subtest 'reject incomplete results on insert' => sub {
};

subtest 'insert a basic record, do not allow incomplete data in update' => sub {
plan tests => 9;
plan tests => 10;

my $id_seq_composition = t::autoqc_util::find_or_save_composition(
$schema, {'id_run' => 1111,
'position' => 1,
'tag_index' => 8});
'tag_index' => 7});
my $values = {
evaluation_results => {"e1"=>1,"e2"=>0},
criteria => {"and"=>["e1","e2"]},
pass => 0,
path => 't/data',
id_seq_composition => $id_seq_composition
};
lives_ok {$schema->resultset($table)->create($values)}
'library type is not defined - no error on insert';

$id_seq_composition = t::autoqc_util::find_or_save_composition(
$schema, {'id_run' => 1111,
'position' => 1,
'tag_index' => 8});
$values = {
evaluation_results => {},
criteria => {},
qc_outcome => {},
Expand Down Expand Up @@ -108,9 +100,8 @@ subtest 'insert a basic record, do not allow incomplete data in update' => sub {
pass => 0,
path => 't/data'
};
throws_ok {$new->update($values)}
qr/Evaluation results present, but library_type absent/,
'library type is not defined - error on update';
lives_ok {$new->update($values)}
'library type is not defined - no error on update';

$values = {
id_seq_composition => $id_seq_composition,
Expand Down Expand Up @@ -277,7 +268,7 @@ subtest 'a full insert/update record with mqc outcome' => sub {
};
my $created=$schema->resultset($table)->create($values);
$rs = $schema->resultset($table)->search({});
is ($rs->count, 2, q[two rows in the table]);
is ($rs->count, 3, q[three rows in the table]);

$mqc_rs = $schema->resultset($mqc_table)->search({id_seq_composition => $id_seq_composition});
is ($mqc_rs->count, 1, 'one mqc record for the entity');
Expand Down
16 changes: 15 additions & 1 deletion t/60-autoqc-checks-review.t
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ subtest 'single expression evaluation' => sub {
};

subtest 'evaluation within the execute method' => sub {
plan tests => 42;
plan tests => 48;

local $ENV{NPG_CACHED_SAMPLESHEET_FILE} =
't/data/autoqc/review/samplesheet_29524.csv';
Expand Down Expand Up @@ -439,6 +439,8 @@ subtest 'evaluation within the execute method' => sub {
foreach my $check (@check_objects) {
lives_ok { $check->execute } 'execute method runs OK';
is ($check->result->pass, 1, 'result pass attribute is set to 1');
is ($check->result->library_type, 'HiSeqX PCR free',
'result library_type attribute is set correctly');
my %expected = map { $_ => 1 } @{$criteria_list};
is_deeply ($check->result->evaluation_results(), \%expected,
'evaluation results are saved');
Expand All @@ -454,7 +456,19 @@ subtest 'evaluation within the execute method' => sub {
$count++;
}

# Undefined library type should not be a problem.
my $lane_lims = (st::api::lims->new(id_run=> 29524)->children())[0];
is ($lane_lims->library_type, undef, 'library type is undefined on lane level');
my $check = npg_qc::autoqc::checks::review->new(
conf_path => $test_data_dir,
qc_in => $test_data_dir,
rpt_list => $rpt_list,
lims => $lane_lims
);
lives_ok { $check->execute } 'execute method runs OK';
is ($check->result->library_type, undef, 'library_type attribute is unset');

$check = npg_qc::autoqc::checks::review->new(
conf_path => "$test_data_dir/unknown_qc_type",
qc_in => $dir,
rpt_list => $rpt_list);
Expand Down

0 comments on commit cb97e0d

Please sign in to comment.