pull from devel to master to create release 68.2.0

Changes

@@ -1,6 +1,15 @@
 LIST OF CHANGES
 ---------------
 
+release 68.2.0
+ - Added '--process_separately_lanes' to the pipeline to explicitly exclude
+   multiple lanes from a merge.
+ - Generalised dir_path method in npg_pipeline::product. This fixed a bug in
+   npg_run_is_deletable, which manifested in wrong expectations about the
+   directory tree for partially merged run data. 
+ - Dropped a check for DRAGEN analysis data from npg_run_is_deletable.
+ - Removed unnecessary tests from t/10-pluggable-central.t
+
 release 68.1.0
  - Apply changes to the code and tests, which follow from removing some
    functionality from npg_tracking::illumina::runfolder, see

lib/npg_pipeline/base.pm

@@ -187,7 +187,10 @@ sub _build_general_values_conf {
 =head2 merge_lanes
 
 Tells p4 stage2 (seq_alignment) to merge all lanes (at their plex level
-if plexed) and to run its downstream tasks using corresponding compositions.
+if plexed, except spiked PhiX and tag_zero).
+
+If not set, this attribute is build lazily. It is set to true for NovaSeq runs,
+which use a Standard flowcell.
 
 =cut
 
@@ -208,6 +211,12 @@ sub _build_merge_lanes {
 
 =head2 merge_by_library
 
+Tells p4 stage2 (seq_alignment) to merge all plexes that belong to the same
+library, except spiked PhiX and tag_zero.
+
+If not set, this attribute is build lazily. It is set to true for indexed
+NovaSeqX runs.
+
 =cut
 
 has q{merge_by_library} => (
@@ -216,13 +225,32 @@ has q{merge_by_library} => (
   lazy_build    => 1,
   documentation => q{Tells p4 stage2 (seq_alignment) to merge all plexes } .
                    q{that belong to the same library, except spiked PhiX and }.
-                   q{tag zero)},
+                   q{tag zero},
 );
 sub _build_merge_by_library {
   my $self = shift;
   return $self->is_indexed && $self->platform_NovaSeqX();
 }
 
+=head2 process_separately_lanes
+
+An array of lane (position) numbers, which should not be merged with anyother
+lanes. To be used in conjunction with C<merge_lanes> or C<merge_by_library>
+attributes. Does not have any impact if both of these attributes are false.
+
+Defaults to an empty array value, meaning that all possible entities will be
+merged. 
+
+=cut
+
+has q{process_separately_lanes} => (
+  isa           => q{ArrayRef},
+  is            => q{ro},
+  default       => sub { return []; },
+  documentation => q{Array of lane numbers, which have to be excluded from } .
+                   q{a merge},
+);
+
 =head2 lims
 
 st::api::lims run-level or product-specific object
@@ -346,7 +374,8 @@ sub _build_products {
 
     if ($self->merge_lanes || $self->merge_by_library) {
 
-      my $all_lims = $self->lims->aggregate_libraries(\@lane_lims);
+      my $all_lims = $self->lims->aggregate_libraries(
+        \@lane_lims, $self->process_separately_lanes);
       @data_lims = @{$all_lims->{'singles'}}; # Might be empty.
 
       # merge_lanes option implies a merge across all lanes.
@@ -425,18 +454,27 @@ sub _lims_object2product {
 sub _check_lane_merge_is_viable {
   my ($self, $lane_lims, $singles, $merges) = @_;
 
+  my %no_merge_lanes = map { $_ => 1 } @{$self->process_separately_lanes};
   my @num_plexes = uniq
-                   map { scalar @{$_} }
-                   map { [grep { !$_->is_control } @{$_}] }
-                   map { [$_->children()] } @{$lane_lims};
+                   map  { scalar @{$_} }
+                   map  { [grep { !$_->is_control } @{$_}] }
+                   map  { [$_->children()] }
+                   grep { ! exists $no_merge_lanes{$_->position} }
+                   @{$lane_lims};
 
   my $m = 'merge_lane option is not viable: ';
   if (@num_plexes > 1) {
     $self->logcroak($m . 'different number of samples in lanes');
   }
-  if (any { !$_->is_control } @{$singles}) {
-    $self->logcroak($m . 'unmerged samples are present after aggregation');
+
+  my @unmerged_unexpected = grep { ! exists $no_merge_lanes{$_->position} }
+                            grep { !$_->is_control }
+                            @{$singles};
+  if (@unmerged_unexpected) {
+    $self->logcroak(
+      $m . 'unexpected unmerged samples are present after aggregation');
   }
+
   if (@{$merges} != $num_plexes[0]) {
     $self->logcroak($m . 'number of merged samples after aggregation ' .
       'differs from the number of samples in a lane');

lib/npg_pipeline/product.pm

@@ -185,7 +185,7 @@ sub _build_rpt_list {
 =head2 selected_lanes
 
 Boolean flag, defaults to false, is meaningful only for a product with multiple
-components. If true, indicates that the conposition does not span all lanes
+components. If true, indicates that the composition does not span all lanes
 of the run.
 
 =cut
@@ -249,8 +249,26 @@ sub file_name {
 
 =head2 dir_path
 
-A relative path for the product, method is inherited from
-npg_tracking::glossary::moniker.
+A relative path for the product. The method is inherited from
+C<npg_tracking::glossary::moniker> and extended here to account for
+a partial merge.
+
+For a merged entity described as C<22:1:24;22:2:24> the directory
+path can be C<plex24> if the run had two lanes, both of which are
+merged, or it can be C<lane1-2/plex24> if the run has more than two
+lanes and this entity is a partial (not over all lanes) merge.
+
+The C<selected_lanes> attribute of this object keeps information
+about the nature of the merge. Its value is passed to the inherited
+C<dir_path> method to ensure that the path is generated correctly.
+
+=cut
+
+around 'dir_path' => sub {
+  my $orig = shift;
+  my $self = shift;
+  return $self->$orig($self->selected_lanes);
+};
 
 =head2 path
 
@@ -263,7 +281,7 @@ generate the path.
 sub path {
   my ($self, $dir) = @_;
   $dir or croak 'Directory argument is needed';
-  return File::Spec->catdir($dir, $self->dir_path($self->selected_lanes));
+  return File::Spec->catdir($dir, $self->dir_path());
 }
 
 =head2 existing_path
@@ -282,7 +300,7 @@ sub existing_path {
   $dir or croak 'Directory argument is needed';
   (-e $dir) or croak "Directory argument $dir does not exist";
 
-  my $path = File::Spec->catdir($dir, $self->dir_path($self->selected_lanes));
+  my $path = File::Spec->catdir($dir, $self->dir_path());
   my $orig = $path;
   if (!-e $path) {
     $path = File::Spec->catdir($dir, $self->generic_name());
@@ -478,7 +496,7 @@ sub lanes_as_products {
 
   if ($with_lims && !$self->has_lims) {
     croak 'In order to use with_lims option this product should have ' .
-	  'lims attribute set';
+          'lims attribute set';
   }
   ##no critic (BuiltinFunctions::ProhibitComplexMappings)
   my @lane_hashes =
@@ -534,13 +552,13 @@ sub subset_as_product {
 
 =head2 chunks_as_product
 
- Interprets the argument integer (required) as the number of chunks to subset
- each product into. Returns a list of product objects with the chunk value
- in each set to one of the values in range 1 .. NUMBER_OF_GIVEN_CHUNKS. See
- chunk_as_product method for details of a product object with the chunk
- attribute defined.
+Interprets the argument integer (required) as the number of chunks to subset
+each product into. Returns a list of product objects with the chunk value
+in each set to one of the values in range 1 .. NUMBER_OF_GIVEN_CHUNKS. See
+chunk_as_product method for details of a product object with the chunk
+attribute defined.
 
- The products in the list are sorted in the accending chunk value order.
+The products in the list are sorted in the accending chunk value order.
 
  my @chunks_p = $p->chunks_as_product(24);
  $p->file_name_root();         # 123_6#4
@@ -568,13 +586,13 @@ sub chunks_as_product {
 
 =head2 chunk_as_product
 
- Interprets the argument integer (required) as the chunk for this product
- to create a new product object for. Returns an object of this class for the chunk with
- a composition identical to the composition object of this object with one exception -
- the chunk value in the object is set to the value of the argument string.
+Interprets the argument integer (required) as the chunk for this product
+to create a new product object for. Returns an object of this class for the chunk with
+a composition identical to the composition object of this object with one exception -
+the chunk value in the object is set to the value of the argument string.
 
- The lims attribute of the returned object is set if the lims attribute of
- this object is set.
+The lims attribute of the returned object is set if the lims attribute of
+this object is set.
 
  my @chunks_p = $p->product_chunk(2);
  $p->file_name_root();         # 123_6#4
@@ -650,7 +668,7 @@ __END__
 
 =head1 LICENSE AND COPYRIGHT
 
-Copyright (C) 2018,2019,2020,2021 Genome Research Ltd.
+Copyright (C) 2018,2019,2020,2021,2024 Genome Research Ltd.
 
 This program is free software: you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by

lib/npg_pipeline/product/release/irods.pm

@@ -202,7 +202,7 @@ sub irods_product_destination_collection_norf {
   $run_collection or croak('Run collection iRODS path is required');
   $product or croak('Product object is required');
   return $per_product_archive
-    ? join q[/], $run_collection, $product->dir_path($product->selected_lanes)
+    ? join q[/], $run_collection, $product->dir_path()
     : $run_collection;
 }
 
@@ -338,7 +338,7 @@ Marina Gourtovaia
 
 =head1 LICENSE AND COPYRIGHT
 
-Copyright (C) 2019,2020,2021,2022 Genome Research Ltd.
+Copyright (C) 2019,2020,2021,2022,2024 Genome Research Ltd.
 
 This program is free software: you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by

lib/npg_pipeline/validation.pm

@@ -470,7 +470,6 @@ sub run {
               $self->_lims_deletable()         &&
               $self->_staging_deletable()      &&
               $self->_irods_seq_deletable()    &&
-              $self->_irods_seq_onboard_deletable() &&
               $self->_autoqc_deletable()       &&
               $self->_file_archive_deletable
                                );
@@ -833,11 +832,6 @@ sub _irods_seq_pp_deletable {
   return $deletable;
 }
 
-sub _irods_seq_onboard_deletable {
-  my $self = shift;
-  return !$self->onboard_analysis_planned();
-}
-
 sub _irods_destination_collection4pp {
   my $self = shift;
 
@@ -1102,7 +1096,7 @@ __END__
 
 =head1 LICENSE AND COPYRIGHT
 
-Copyright (C) 2019,2020,2021,2022,2023 Genome Research Ltd.
+Copyright (C) 2019,2020,2021,2022 Genome Research Ltd.
 
 This program is free software: you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by

t/10-base.t

@@ -64,7 +64,7 @@ subtest 'repository preexec' => sub {
 };
 
 subtest 'products - merging (or not) lanes' => sub {
-  plan tests => 19;
+  plan tests => 22;
 
   my $rf_path = q[t/data/novaseqx/20231017_LH00210_0012_B22FCNFLT3];
   my $b = npg_pipeline::base->new(runfolder_path => $rf_path, id_run => 47995);
@@ -79,12 +79,23 @@ subtest 'products - merging (or not) lanes' => sub {
   cp 't/data/novaseq/210111_A00513_0447_AHJ55JDSXY/RunInfo.xml',  "$rf_path/RunInfo.xml";
   $b = npg_pipeline::base->new(runfolder_path => $rf_path, id_run => 999);
   ok ($b->merge_lanes, 'merge_lanes flag is set');
+  ok (!$b->_selected_lanes, 'selected_lanes flag is not set');
   lives_ok {$products = $b->products} 'products hash created for NovaSeq run';
   ok (exists $products->{'lanes'}, 'products lanes key exists');
   is (scalar @{$products->{'lanes'}}, 4, 'four lane product');
   ok (exists $products->{'data_products'}, 'products data_products key exists');
   is (scalar @{$products->{'data_products'}}, 29, '29 data products'); 
 
+  $b = npg_pipeline::base->new(
+    runfolder_path => $rf_path,
+    id_run => 999,
+    merge_lanes => 1,
+    process_separately_lanes => [2]
+  );
+  # 8 products out of previous 29 are tag zero and spiked phiX
+  is (scalar @{$b->products->{'data_products'}}, 50, '50 data products');
+  ok ($b->_selected_lanes, 'selected_lanes flag is set');
+
   local $ENV{NPG_CACHED_SAMPLESHEET_FILE} = 't/data/products/samplesheet_rapidrun_nopool.csv';
   cp 't/data/run_params/runParameters.hiseq.rr.xml',  "$rf_path/runParameters.xml";
   cp 't/data/run_params/RunInfo.hiseq.rr.xml',  "$rf_path/RunInfo.xml"; 
@@ -109,7 +120,7 @@ subtest 'products - merging (or not) lanes' => sub {
 };
 
 subtest 'products - merging (or not) libraries' => sub {
-  plan tests => 418;
+  plan tests => 423;
 
   my $rf_info = $util->create_runfolder();
   my $rf_path = $rf_info->{'runfolder_path'};
@@ -173,6 +184,22 @@ subtest 'products - merging (or not) libraries' => sub {
     ok ($p->selected_lanes, 'selected_lanes flag is set to true');
   }
 
+  $b = npg_pipeline::base->new(
+    runfolder_path => $rf_path,
+    id_run => $id_run,
+    process_separately_lanes => [1,2,5,6]
+  );
+  @products = @{$b->products()->{'data_products'}};
+  is (@products, 142, 'number of data products is 142');
+
+  $b = npg_pipeline::base->new(
+    runfolder_path => $rf_path,
+    id_run => $id_run,
+    process_separately_lanes => [1,6]
+  );
+  @products = @{$b->products()->{'data_products'}};
+  is (@products, 142, 'number of data products is 142');
+
   # Expect lanes 3 and 4 merged.
   $b = npg_pipeline::base->new(
     runfolder_path => $rf_path, id_run => $id_run, lanes => [4,8,3]);
@@ -236,6 +263,28 @@ subtest 'products - merging (or not) libraries' => sub {
     }
   }
   is (@products, 0, 'no products are left');
+
+  # remove lane 3 from the merge - no merge will take place
+  $b = npg_pipeline::base->new(
+    runfolder_path => $rf_path,
+    id_run => $id_run,
+    lanes => [4,8,3],
+    merge_by_library => 1,
+    process_separately_lanes => [3]
+  );
+  @products = @{$b->products()->{'data_products'}};
+  is (@products, 64, 'number of data products is 64');
+
+  $b = npg_pipeline::base->new(
+    runfolder_path => $rf_path,
+    id_run => $id_run,
+    lanes => [4,8,3],
+    merge_by_library => 0,
+    process_separately_lanes => [3,8]
+  );
+  lives_ok { @products = @{$b->products()->{'data_products'}} }
+    'process_separately_lanes is compatible with suppressed merge';
+  is (@products, 64, 'number of data products is 64');
 };
 
 sub _generate_rpt {