From 3ff125954036261aca84e6924158fff3e5f034b8 Mon Sep 17 00:00:00 2001 From: Marina Gourtovaia Date: Tue, 29 Aug 2023 18:05:49 +0100 Subject: [PATCH 1/4] Switched default LIMS driver in samplesheet generation Switched the default LIMS driver in samplesheet generation from 'xml' to 'ml_warehouse'. Added 'lims_driver_type' attribute to npg::samplesheet. Updated the tests for samplesheet generation, which are using the xml LIMS driver, to set the driver type explicitly. --- Changes | 6 ++++++ lib/npg/samplesheet.pm | 18 +++++++++++++++--- t/47-samplesheet.t | 32 ++++++++++++++++---------------- 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/Changes b/Changes index 319702e2..65a94094 100644 --- a/Changes +++ b/Changes @@ -1,5 +1,11 @@ LIST OF CHANGES + - Switched the default LIMS driver in samplesheet generation from 'xml' + to 'ml_warehouse'. + - Added 'lims_driver_type' attribute to npg::samplesheet. + - Updated the tests for samplesheet generation, which are using the xml + LIMS driver, to set the driver type explicitly. + release 96.0.0 - Fixed a regression in the npg_move_runfolder script, which made it unusable. diff --git a/lib/npg/samplesheet.pm b/lib/npg/samplesheet.pm index cbed3db2..0008d5f7 100755 --- a/lib/npg/samplesheet.pm +++ b/lib/npg/samplesheet.pm @@ -62,12 +62,25 @@ still retained in the relevant custom fields. my$config=get_config_staging_areas(); Readonly::Scalar my $SAMPLESHEET_PATH => $config->{'samplesheets'}||q(samplesheets/); Readonly::Scalar my $MIN_COLUMN_NUM => 3; -Readonly::Scalar my $DEFAULT_LIMS_DRIVER_TYPE => 'xml'; +Readonly::Scalar my $DEFAULT_LIMS_DRIVER_TYPE => 'ml_warehouse'; ################################################################## ####################### Public attributes ######################## ################################################################## +=head2 lims_driver_type + +LIMs driver type to use, defaults to ml_warehouse. + +=cut + +has 'lims_driver_type' => ( + 'isa' => 'Str', + 'required' => 0, + 'is' => 'ro', + 'default' => $DEFAULT_LIMS_DRIVER_TYPE, +); + =head2 id_run An optional attribute @@ -180,7 +193,6 @@ An attribute, an array of st::api::lims type objects. This attribute should normally be provided by the caller via the constuctor. If the attribute is not provided, it it built automatically. -XML st::api::lims driver is used to access LIMS data. =cut @@ -201,7 +213,7 @@ sub _build_lims { } return [st::api::lims->new( batch_id => $id, - driver_type => $DEFAULT_LIMS_DRIVER_TYPE)->children]; + driver_type => $self->lims_driver_type)->children]; }; =head2 output diff --git a/t/47-samplesheet.t b/t/47-samplesheet.t index c8a5964e..9e676ab7 100644 --- a/t/47-samplesheet.t +++ b/t/47-samplesheet.t @@ -23,18 +23,18 @@ subtest 'object creation' => sub { plan tests => 8; my $result = q(); - dies_ok { npg::samplesheet->new( repository=>$dir, output=>\$result)->process } + dies_ok { npg::samplesheet->new( lims_driver_type=>'xml', repository=>$dir, output=>\$result)->process } 'sample sheet process fails when neither run object nor id_run given'; my $ss; - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007); } 'sample sheet object - no output provided'; + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007); } 'sample sheet object - no output provided'; cmp_ok($ss->output, 'eq', '/nfs/sf49/ILorHSorMS_sf49/samplesheets/wibble/MS0001309-300.csv', 'default output location (with zeroes trimmed appropriately)'); is($ss->lims->[0]->driver_type, 'xml', 'xml driver is used'); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946); } 'sample sheet object - no output provided'; + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946); } 'sample sheet object - no output provided'; cmp_ok($ss->output, 'eq', '/nfs/sf49/ILorHSorMS_sf49/samplesheets/wibble/000000000-A0616.csv', 'default output location'); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007); } 'sample sheet object - no output provided'; + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007); } 'sample sheet object - no output provided'; my $orig_flowcell_id = $ss->run->flowcell_id; $ss->run->flowcell_id(q(MS2000132-500V2)); cmp_ok($ss->output, 'eq', '/nfs/sf49/ILorHSorMS_sf49/samplesheets/wibble/MS2000132-500V2.csv', 'default output location copes with V2 MiSeq cartirdges/reagent kits'); @@ -91,13 +91,13 @@ RESULT_7007 my $ss; my $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007, output=>\$result); } 'sample sheet object for unplexed paired run'; + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007, output=>\$result); } 'sample sheet object for unplexed paired run'; lives_ok { $ss->process(); } ' sample sheet generated'; is_string($result, $expected_result_7007); my $run = $schema->resultset(q(Run))->find(7007); $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, run=>$run, output=>\$result); } 'sample sheet object from run object - no id_run given'; + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, run=>$run, output=>\$result); } 'sample sheet object from run object - no id_run given'; lives_ok { $ss->process(); } ' sample sheet generated'; is_string($result, $expected_result_7007); }; @@ -107,7 +107,7 @@ subtest 'default samplesheet for a plexed paired run' => sub { my $ss; my $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, output=>\$result); } 'samplesheet object for plexed paired run'; + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, output=>\$result); } 'samplesheet object for plexed paired run'; my $expected_result = << 'RESULT_6946'; [Header],,,, Investigator Name,mq1,,, @@ -150,7 +150,7 @@ subtest 'default samplesheet for a plexed paired run with reference fallback' => my $ss; my $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7825, output=>\$result); } 'sample sheet object for plexed paired run'; + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7825, output=>\$result); } 'sample sheet object for plexed paired run'; my $expected_result = << 'RESULT_7825'; [Header],,,, Investigator Name,nh4,,, @@ -187,7 +187,7 @@ subtest 'default samplesheet, mkfastq option enabled' => sub { # with the mkfastq option we get an extra leading column, Lane my $ss; my $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7826, mkfastq => 1, output=>\$result); } + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7826, mkfastq => 1, output=>\$result); } 'sample sheet object mkfastq'; my $expected_result = << 'RESULT_mkfastq'; [Header],,,,, @@ -224,7 +224,7 @@ subtest 'default samplesheet for dual index' => sub { my $ss; my $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7826, output=>\$result); } 'sample sheet object for dual index'; + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7826, output=>\$result); } 'sample sheet object for dual index'; my $expected_result = << 'RESULT_7826'; [Header],,,, Investigator Name,nh4,,, @@ -260,13 +260,13 @@ subtest 'extended samplesheets' => sub { my $ss; my $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, extend => 1, id_run=>7007, output=>\$result); } 'extended sample sheet object for unplexed paired run'; + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, extend => 1, id_run=>7007, output=>\$result); } 'extended sample sheet object for unplexed paired run'; ok(!$ss->_dual_index, 'no dual index'); lives_ok { $ss->process(); } ' sample sheet generated'; is_string($result, read_file('t/data/samplesheet/7007_extended.csv')); $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } 'extended sample sheet object for plexed paired run'; + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } 'extended sample sheet object for plexed paired run'; ok(!$ss->_dual_index, 'no dual index'); lives_ok { $ss->process(); } ' sample sheet generated'; is_string($result, read_file('t/data/samplesheet/6946_extended.csv')); @@ -276,7 +276,7 @@ subtest 'extended samplesheets' => sub { $schema->resultset('Run')->find(6946)->update({batch_id => 4775}); $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } 'extended sample sheet object for unplexed paired 8 lane run with a control lane'; lives_ok { $ss->process(); } 'sample sheet generated'; is_string($result, read_file('t/data/samplesheet/1control7libs_extended.csv')); @@ -286,7 +286,7 @@ subtest 'extended samplesheets' => sub { $schema->resultset('Run')->find(6946)->update({batch_id => 16249}); $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } 'extended sample sheet object for plexed paired 8 lane run'; ok(!$ss->_dual_index, 'no dual index'); lives_ok { $ss->process(); } 'sample sheet generated'; @@ -297,7 +297,7 @@ subtest 'extended samplesheets' => sub { $schema->resultset('Run')->find(6946)->update({batch_id => 23798}); $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } 'extended sample sheet object for plexed paired run with both pool and library lanes'; ok($ss->_dual_index, 'dual index from a 16 char first index'); lives_ok { $ss->process(); } 'sample sheet generated'; @@ -307,7 +307,7 @@ subtest 'extended samplesheets' => sub { $schema->resultset('Run')->find(6946)->update({batch_id => 1,}); $result = q(); - lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } + lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } 'extended sample sheet object for plexed paired run with both pool and library lanes'; ok($ss->_dual_index, 'dual index from two indexes in LIMs'); lives_ok { $ss->process(); } 'sample sheet generated'; From 19e1f371598ca68dc52fd794f18130467e17ccc6 Mon Sep 17 00:00:00 2001 From: Marina Gourtovaia Date: Wed, 30 Aug 2023 11:44:12 +0100 Subject: [PATCH 2/4] Dropped a special case of batch-less pools --- Changes | 2 ++ lib/npg/samplesheet.pm | 7 ------- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/Changes b/Changes index 65a94094..c5a1422d 100644 --- a/Changes +++ b/Changes @@ -5,6 +5,8 @@ LIST OF CHANGES - Added 'lims_driver_type' attribute to npg::samplesheet. - Updated the tests for samplesheet generation, which are using the xml LIMS driver, to set the driver type explicitly. + - In samplesheet generation, dropped a special case of MiSeq instruments + without a batch; last use in production was over 5 years ago. release 96.0.0 - Fixed a regression in the npg_move_runfolder script, diff --git a/lib/npg/samplesheet.pm b/lib/npg/samplesheet.pm index 0008d5f7..5d7f3873 100755 --- a/lib/npg/samplesheet.pm +++ b/lib/npg/samplesheet.pm @@ -204,13 +204,6 @@ has 'lims' => ( sub _build_lims { my $self=shift; my $id = $self->run->batch_id; - if ($id=~/\A\d{13}\z/smx) { - load_class 'st::api::lims::warehouse'; - return [st::api::lims->new( - position => 1, - driver => st::api::lims::warehouse->new(position=>1, tube_ean13_barcode=>$id) - )]; - } return [st::api::lims->new( batch_id => $id, driver_type => $self->lims_driver_type)->children]; From 626bd65f94ed2258f9ade06960e374d88355bff2 Mon Sep 17 00:00:00 2001 From: Marina Gourtovaia Date: Wed, 30 Aug 2023 12:40:57 +0100 Subject: [PATCH 3/4] Fixed method for building lims objects --- lib/npg/samplesheet.pm | 38 +++++++++++++++++++++++++++++++++----- t/47-samplesheet.t | 42 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/lib/npg/samplesheet.pm b/lib/npg/samplesheet.pm index 5d7f3873..85e69743 100755 --- a/lib/npg/samplesheet.pm +++ b/lib/npg/samplesheet.pm @@ -15,6 +15,7 @@ use st::api::lims; use st::api::lims::samplesheet; use npg_tracking::util::config qw(get_config_staging_areas); use npg_tracking::util::abs_path qw(abs_path); +use WTSI::DNAP::Warehouse::Schema; with 'npg_tracking::glossary::run'; @@ -170,6 +171,21 @@ sub _build_npg_tracking_schema { return $s } +=head2 mlwh_schema + +DBIx schema class for ml_warehouse access. + +=cut + +has 'mlwh_schema' => ( + isa => 'WTSI::DNAP::Warehouse::Schema', + is => 'ro', + required => 0, + lazy_build => 1,); +sub _build_mlwh_schema { + return WTSI::DNAP::Warehouse::Schema->connect(); +} + =head2 run An attribute, DBIx object for a row in the run table of the tracking database. @@ -203,10 +219,20 @@ has 'lims' => ( ); sub _build_lims { my $self=shift; - my $id = $self->run->batch_id; - return [st::api::lims->new( - batch_id => $id, - driver_type => $self->lims_driver_type)->children]; + + my $ref = {driver_type => $self->lims_driver_type}; + my $batch_id = $self->run->batch_id; + if ($self->lims_driver_type eq $DEFAULT_LIMS_DRIVER_TYPE) { + $ref->{'id_flowcell_lims'} = $batch_id; + $ref->{'mlwh_schema'} = $self->mlwh_schema; + } elsif ($self->lims_driver_type eq 'xml') { + $ref->{'batch_id'} = $batch_id; + } else { + croak sprintf 'Lazy-build for driver type %s is not inplemented', + $self->lims_driver_type; + } + + return [st::api::lims->new($ref)->children]; }; =head2 output @@ -597,6 +623,8 @@ __END__ =item open +=item WTSI::DNAP::Warehouse::Schema + =back =head1 INCOMPATIBILITIES @@ -609,7 +637,7 @@ David K. Jackson Edavid.jackson@sanger.ac.ukE =head1 LICENSE AND COPYRIGHT -Copyright (C) 2019,2020 Genome Research Ltd. +Copyright (C) 2019,2020, 2023 Genome Research Ltd. This file is part of NPG. diff --git a/t/47-samplesheet.t b/t/47-samplesheet.t index 9e676ab7..4b88dea3 100644 --- a/t/47-samplesheet.t +++ b/t/47-samplesheet.t @@ -1,11 +1,12 @@ use strict; use warnings; -use Test::More tests => 11; +use Test::More tests => 13; use Test::LongString; use Test::Exception; use File::Slurp; use File::Temp qw/tempdir/; use File::Path qw/make_path/; +use Moose::Meta::Class; use t::dbic_util; local $ENV{'dev'} = q(wibble); # ensure we're not going live anywhere @@ -15,6 +16,12 @@ use_ok('npg::samplesheet'); use_ok('st::api::lims'); my $schema = t::dbic_util->new->test_schema(); + +my $class = Moose::Meta::Class->create_anon_class(roles=>[qw/npg_testing::db/]); +my $mlwh_schema = $class->new_object({})->create_test_db( + q[WTSI::DNAP::Warehouse::Schema], q[t/data/fixtures_lims_wh] +); + local $ENV{NPG_WEBSERVICE_CACHE_DIR} = q(t/data/samplesheet); my $dir = tempdir( CLEANUP => 1 ); @@ -40,6 +47,39 @@ subtest 'object creation' => sub { cmp_ok($ss->output, 'eq', '/nfs/sf49/ILorHSorMS_sf49/samplesheets/wibble/MS2000132-500V2.csv', 'default output location copes with V2 MiSeq cartirdges/reagent kits'); }; +subtest 'error on an unknown driver types' => sub { + plan tests => 1; + + throws_ok { + npg::samplesheet->new( + lims_driver_type => 'foo', + repository => $dir, + npg_tracking_schema => $schema, + id_run => 7007)->lims() + } qr/Lazy-build for driver type foo is not inplemented/, + 'error with the driver type for which LIMS objects cannot be built'; +}; + +subtest 'simple tests for the default driver' => sub { + plan tests => 2; + + my $run_row = $schema->resultset('Run')->find(7007); + my $current_batch_id = $run_row->batch_id; + $run_row->update({batch_id => 57543}); + + my $ss = npg::samplesheet->new( + repository => $dir, + npg_tracking_schema => $schema, + mlwh_schema => $mlwh_schema, + id_run => 7007 + ); + is ($ss->lims_driver_type, 'ml_warehouse', 'correct default driver type'); + my $lims = $ss->lims(); + is (@{$lims}, 1, 'LIMS data for 1 lane is built'); + + $run_row->update({batch_id => $current_batch_id}); +}; + subtest 'values conversion' => sub { plan tests => 12; From 8b15569a82d9703557171106b082a243accc6a85 Mon Sep 17 00:00:00 2001 From: Marina Gourtovaia Date: Thu, 31 Aug 2023 13:50:54 +0100 Subject: [PATCH 4/4] Fixed indentation and added quoting for properties --- lib/npg/samplesheet.pm | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/npg/samplesheet.pm b/lib/npg/samplesheet.pm index 85e69743..4f7d626c 100755 --- a/lib/npg/samplesheet.pm +++ b/lib/npg/samplesheet.pm @@ -178,10 +178,11 @@ DBIx schema class for ml_warehouse access. =cut has 'mlwh_schema' => ( - isa => 'WTSI::DNAP::Warehouse::Schema', - is => 'ro', - required => 0, - lazy_build => 1,); + 'isa' => 'WTSI::DNAP::Warehouse::Schema', + 'is' => 'ro', + 'required' => 0, + 'lazy_build' => 1, +); sub _build_mlwh_schema { return WTSI::DNAP::Warehouse::Schema->connect(); }