Skip to content

Commit 2df0cfb

Browse files
authored
Merge pull request wtsi-npg#870 from wtsi-npg/devel
pull from devel to master to create release 68.7.0
2 parents 7f27799 + b42899a commit 2df0cfb

File tree

8 files changed

+228
-168
lines changed

8 files changed

+228
-168
lines changed

.github/workflows/run-tests.yml

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ jobs:
1111
shell: bash -l -e -o pipefail {0}
1212

1313
env:
14-
PERL_CACHE: ~/perl5 # Perlbrew and CPAN modules installed here, cached
15-
NPG_LIB: ~/perl5npg # NPG modules installed here, not cached
16-
WSI_CONDA_CHANNEL: "https://dnap.cog.sanger.ac.uk/npg/conda/devel/generic"
14+
PERL_CACHE: $HOME/perl5 # Perlbrew and CPAN modules installed here, cached
15+
NPG_LIB: $HOME/perl5npg # NPG modules installed here, not cached
16+
WSI_CONDA_CHANNEL: https://dnap.cog.sanger.ac.uk/npg/conda/devel/generic
17+
CONDA_HOME: $HOME/conda
1718
CONDA_TEST_ENV: test-environment
1819
WTSI_NPG_GITHUB_URL: https://github.com/wtsi-npg
1920
WTSI_NPG_BUILD_BRANCH: ${{ github.base_ref || github.ref }}
@@ -24,6 +25,8 @@ jobs:
2425

2526
steps:
2627
- uses: actions/checkout@v4
28+
with:
29+
fetch-depth: '0'
2730

2831
- name: "Install OS dependencies"
2932
run: |
@@ -32,22 +35,32 @@ jobs:
3235
sudo apt-get remove -y nginx libgd3
3336
sudo apt-get install -y libgd-dev uuid-dev libgd-text-perl
3437
35-
- name: "Initialize Miniconda"
38+
- name: "Install Miniforge"
3639
run: |
37-
echo 'source $CONDA/etc/profile.d/conda.sh' >> "$HOME/.bash_profile"
40+
CONDA_HOME=${{ env.CONDA_HOME }} ./scripts/install_miniforge.sh
41+
echo 'source "${{ env.CONDA_HOME }}/etc/profile.d/conda.sh"' >> "$HOME/.bash_profile"
42+
43+
- name: "Cache Conda"
44+
id: miniforgeCache
45+
uses: actions/cache@v4
46+
with:
47+
path: |
48+
${{ env.CONDA_HOME }}/pkgs
49+
${{ env.CONDA_HOME }}/envs
50+
key: ${{ runner.os }}-build-miniforge
3851

3952
- name: "Install Conda packages"
4053
run: |
41-
conda config --prepend pkgs_dirs ~/conda/pkgs
42-
conda config --prepend envs_dirs ~/conda/envs
54+
conda config --prepend pkgs_dirs ${{ env.CONDA_HOME }}/pkgs
55+
conda config --prepend envs_dirs ${{ env.CONDA_HOME }}/envs
4356
4457
conda config --set auto_update_conda False
45-
conda config --prepend channels "$WSI_CONDA_CHANNEL"
58+
conda config --prepend channels "${{ env.WSI_CONDA_CHANNEL }}"
4659
conda info
4760
48-
conda create -y -n "$CONDA_TEST_ENV"
49-
conda install -y -n "$CONDA_TEST_ENV" baton
50-
conda install -y -n "$CONDA_TEST_ENV" samtools
61+
conda create -y -n "${{ env.CONDA_TEST_ENV }}"
62+
conda install -y -n "${{ env.CONDA_TEST_ENV }}" baton
63+
conda install -y -n "${{ env.CONDA_TEST_ENV }}" samtools
5164
5265
- name: "Cache Perl"
5366
id: cache-perl
@@ -77,14 +90,14 @@ jobs:
7790
- name: "Install Perl dependencies"
7891
run: |
7992
cpanm --local-lib=${{ env.PERL_CACHE }} local::lib
80-
eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib="$NPG_LIB")
93+
eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib="${{ env.NPG_LIB }}")
8194
eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib)
8295
8396
cpanm --quiet --notest Module::Build
8497
cpanm --quiet --notest Net::SSLeay
8598
cpanm --quiet --notest https://github.com/chapmanb/vcftools-cpan/archive/v0.953.tar.gz
8699
87-
./scripts/install_wsi_dependencies.sh "$NPG_LIB" \
100+
./scripts/install_wsi_dependencies.sh "${{ env.NPG_LIB }}" \
88101
perl-dnap-utilities \
89102
perl-irods-wrap \
90103
ml_warehouse \
@@ -98,23 +111,23 @@ jobs:
98111
- name: "Log install failure"
99112
if: ${{ failure() }}
100113
run: |
101-
find ~/.cpanm/work -cmin -1 -name '*.log' -exec tail -n20 {} \;
114+
find $HOME/.cpanm/work -cmin -1 -name '*.log' -exec tail -n20 {} \;
102115
103116
- name: "Archive CPAN logs on failure"
104117
if: ${{ failure() }}
105118
uses: actions/upload-artifact@v4
106119
with:
107120
name: cpan_log
108-
path: ~/.cpanm/work/*/build.log
121+
path: $HOME/.cpanm/work/*/build.log
109122
retention-days: 5
110123

111124
- name: "Run tests"
112125
run: |
113-
conda activate "$CONDA_TEST_ENV"
126+
conda activate "${{ env.CONDA_TEST_ENV }}"
114127
conda info --envs
115128
116129
eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib)
117-
eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib="$NPG_LIB")
130+
eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib="${{ env.NPG_LIB }}")
118131
119132
export TEST_AUTHOR=1
120133
perl Build.PL

Changes

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
LIST OF CHANGES
22
---------------
33

4+
release 68.7.0 (2024-12-02)
5+
- npg_pipeline::function::autoqc
6+
- Simplified the flow of the code.
7+
- Made clearer logic for choosing QC checks to run, provided comments.
8+
- Allowed for running the review QC check on lanes.
9+
- Move from Miniconda to Miniforge
10+
411
release 68.6.0 (2024-10-24)
512
- The runfolder_path attribute is passed to the constructor of the review
613
autoqc check object when deciding whether this check should run.

MANIFEST

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,6 +1042,7 @@ t/data/release/config/notify_off/product_release.yml
10421042
t/data/release/config/notify_on/product_release.yml
10431043
t/data/release/config/qc_review/product_release.yml
10441044
t/data/release/config/qc_review_default/product_release.yml
1045+
t/data/release/config/qc_review_default_with_lane/product_release.yml
10451046
t/data/release/config/pp_archival/product_release.yml
10461047
t/data/release/config/pp_archival/product_release_no_array.yml
10471048
t/data/release/config/pp_archival/product_release_no_filters.yml

lib/npg_pipeline/function/autoqc.pm

Lines changed: 77 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,23 @@ with q{npg_pipeline::function::util};
1919

2020
our $VERSION = '0';
2121

22-
Readonly::Scalar my $QC_SCRIPT_NAME => q{qc};
22+
Readonly::Scalar my $QC_SCRIPT_NAME => q{qc};
23+
2324
Readonly::Array my @CHECKS_NEED_PAIREDNESS_INFO => qw/
2425
insert_size
2526
gc_fraction
2627
qX_yield
2728
/;
2829

30+
Readonly::Scalar my $TARGET_FILE_CHECK_RE => qr{ \A
31+
adapter |
32+
bcfstats |
33+
verify_bam_id |
34+
genotype |
35+
pulldown_metrics |
36+
haplotag_metrics
37+
\Z }xms;
38+
2939
has q{qc_to_run} => (isa => q{Str},
3040
is => q{ro},
3141
required => 1,);
@@ -51,47 +61,6 @@ sub _build__check_uses_refrepos {
5161
->find_attribute_by_name('repository') ? 1 : 0;
5262
}
5363

54-
has q{_is_lane_level_check} => (
55-
isa => q{Bool},
56-
is => q{ro},
57-
required => 0,
58-
init_arg => undef,
59-
lazy_build => 1,);
60-
sub _build__is_lane_level_check {
61-
my $self = shift;
62-
return $self->qc_to_run() =~ /^ spatial_filter $/smx;
63-
}
64-
65-
has q{_is_lane_level_check4indexed_lane} => (
66-
isa => q{Bool},
67-
is => q{ro},
68-
required => 0,
69-
init_arg => undef,
70-
lazy_build => 1,);
71-
sub _build__is_lane_level_check4indexed_lane {
72-
my $self = shift;
73-
return $self->qc_to_run() =~ /^ tag_metrics $/smx;
74-
}
75-
76-
has q{_is_check4target_file} => (
77-
isa => q{Bool},
78-
is => q{ro},
79-
required => 0,
80-
init_arg => undef,
81-
lazy_build => 1,);
82-
sub _build__is_check4target_file {
83-
my $self = shift;
84-
##no critic (RegularExpressions::RequireBracesForMultiline)
85-
##no critic (RegularExpressions::ProhibitComplexRegexes)
86-
return $self->qc_to_run() =~ /^ adapter |
87-
bcfstats |
88-
verify_bam_id |
89-
genotype |
90-
pulldown_metrics |
91-
haplotag_metrics |
92-
review $/smx;
93-
}
94-
9564
sub BUILD {
9665
my $self = shift;
9766
load_class($self->_qc_module_name);
@@ -111,15 +80,19 @@ sub create {
11180
push @definitions, $self->_create_definition4interop();
11281
} else {
11382

83+
my $is_lane = 1;
11484
for my $lp (@{$self->products->{lanes}}) {
11585

11686
$self->debug(sprintf ' autoqc check %s for lane, rpt_list: %s, is_pool: %s',
11787
$self->qc_to_run(), $lp->rpt_list, ($lp->lims->is_pool? q[True]: q[False]));
11888

11989
$done_as_lane{$lp->rpt_list} = 1;
120-
push @definitions, $self->_create_definition($lp, 0); # is_plex is always 0 here
90+
if ($self->_should_run($lp, $is_lane)) {
91+
push @definitions, $self->_create_definition($lp);
92+
}
12193
}
12294

95+
$is_lane = 0;
12396
for my $dp (@{$self->products->{data_products}}) {
12497
# skip data_products that have already been processed as lanes
12598
# (i.e. libraries or single-sample pools)
@@ -133,7 +106,9 @@ sub create {
133106
$self->qc_to_run(), $dp->{rpt_list}, ($is_plex? q[True]: q[False]),
134107
($dp->lims->is_pool? q[True]: q[False]), ($is_plex? $tag_index: q[NONE]));
135108

136-
push @definitions, $self->_create_definition($dp, $is_plex);
109+
if ($self->_should_run($dp, $is_lane, $is_plex)) {
110+
push @definitions, $self->_create_definition($dp);
111+
}
137112
}
138113
}
139114

@@ -145,14 +120,17 @@ sub create {
145120
}
146121

147122
sub _create_definition {
148-
my ($self, $product, $is_plex) = @_;
149-
150-
if ($self->_should_run($is_plex, $product)) {
151-
my $command = $self->_generate_command($product);
152-
return $self->_create_definition_object($product, $command);
123+
my ($self, $product) = @_;
124+
125+
my $ref = {
126+
'job_name' => $self->_job_name(),
127+
'composition' => $product->composition,
128+
'command' => $self->_generate_command($product)
129+
};
130+
if ( ($self->qc_to_run eq 'adapter') || $self->_check_uses_refrepos() ) {
131+
$ref->{'command_preexec'} = $self->repos_pre_exec_string();
153132
}
154-
155-
return;
133+
return $self->create_definition($ref);
156134
}
157135

158136
sub _job_name {
@@ -190,23 +168,6 @@ sub _create_definition4interop {
190168
return $self->create_definition($ref);
191169
}
192170

193-
sub _create_definition_object {
194-
my ($self, $product, $command) = @_;
195-
196-
my $ref = {};
197-
my $qc_to_run = $self->qc_to_run;
198-
199-
$ref->{'job_name'} = $self->_job_name();
200-
$ref->{'composition'} = $product->composition;
201-
$ref->{'command'} = $command;
202-
203-
if ( ($qc_to_run eq 'adapter') || $self->_check_uses_refrepos() ) {
204-
$ref->{'command_preexec'} = $self->repos_pre_exec_string();
205-
}
206-
207-
return $self->create_definition($ref);
208-
}
209-
210171
sub _generate_command {
211172
my ($self, $dp) = @_;
212173

@@ -282,64 +243,64 @@ sub _generate_command {
282243
}
283244

284245
sub _should_run {
285-
my ($self, $is_plex, $product) = @_;
286-
287-
my $can_run = 1;
246+
my ($self, $product, $is_lane, $is_plex) = @_;
288247

289-
my $is_lane = !$is_plex; # if it's not a plex, it's a lane
290-
my $rpt_list = $product->rpt_list;
291-
my $is_pool = $product->lims->is_pool;
292-
my $is_tag_zero = $product->is_tag_zero_product;
248+
my $is_pool = $product->lims->is_pool; # Note - true for merged data.
293249

294-
if($self->qc_to_run() eq 'spatial_filter' and ($self->platform_NovaSeq or $self->platform_NovaSeqX)) {
295-
return 0;
250+
if ($self->qc_to_run() eq 'spatial_filter') {
251+
# Run on individual lanes only.
252+
return $is_lane && !($self->platform_NovaSeq || $self->platform_NovaSeqX) ? 1 : 0;
296253
}
297-
298-
if ($self->_is_lane_level_check()) {
299-
return !$is_plex;
254+
if ($self->qc_to_run() eq 'tag_metrics') {
255+
# For indexed runs, run on individual lanes if they are pools of samples.
256+
return $self->is_indexed() && $is_lane && $is_pool ? 1 : 0;
300257
}
301-
302-
if ($self->_is_lane_level_check4indexed_lane()) {
303-
return $is_lane && $is_pool;
258+
if ($self->qc_to_run() eq 'review') {
259+
# This check should never run on tag zero, other entities are fair game.
260+
if ($is_plex && $product->is_tag_zero_product) {
261+
return 0;
262+
}
304263
}
305-
306-
if ($self->_is_check4target_file()) {
307-
$can_run = (($is_lane && !$is_pool) ||
308-
($is_plex && !$is_tag_zero));
264+
if ($self->qc_to_run() =~ $TARGET_FILE_CHECK_RE) {
265+
# Do not run on tag zero files, unmerged or merged.
266+
# Do not run on lane-level data if the lane is a pool.
267+
# Run on lane-level data for a single library.
268+
# Run on merged plexes or lanes.
269+
if (($is_plex && $product->is_tag_zero_product) || ($is_lane && $is_pool)) {
270+
return 0;
271+
}
309272
}
310273

311-
if ($can_run) {
312-
my %init_hash = ( rpt_list => $rpt_list, lims => $product->lims );
313-
314-
if ($self->has_repository && $self->_check_uses_refrepos()) {
315-
$init_hash{'repository'} = $self->repository;
316-
}
317-
if ($self->qc_to_run() eq 'insert_size') {
318-
$init_hash{'is_paired_read'} = $self->is_paired_read() ? 1 : 0;
319-
} elsif ($self->qc_to_run() eq 'review') {
320-
$init_hash{'product_conf_file_path'} = $self->product_conf_file_path;
321-
$init_hash{'runfolder_path'} = $self->runfolder_path;
322-
} elsif ($self->qc_to_run() eq 'genotype') {
323-
my $ref_fasta = npg_pipeline::cache::reference->instance()
324-
->get_path($product, q(fasta), $self->repository());
325-
if ($ref_fasta) {
326-
$init_hash{'reference_fasta'} = $ref_fasta;
327-
}
274+
# The rest will abide by the return value of the can_run method of the
275+
# autoqc check. We have to provide enough argument to the constructor for
276+
# the object to be created and can_run to execute.
277+
my %init_hash = ( rpt_list => $product->rpt_list, lims => $product->lims );
278+
if ($self->has_repository && $self->_check_uses_refrepos()) {
279+
$init_hash{'repository'} = $self->repository;
280+
}
281+
if ($self->qc_to_run() eq 'insert_size') {
282+
$init_hash{'is_paired_read'} = $self->is_paired_read() ? 1 : 0;
283+
} elsif ($self->qc_to_run() eq 'review') {
284+
$init_hash{'product_conf_file_path'} = $self->product_conf_file_path;
285+
$init_hash{'runfolder_path'} = $self->runfolder_path;
286+
} elsif ($self->qc_to_run() eq 'genotype') {
287+
my $ref_fasta = npg_pipeline::cache::reference->instance()
288+
->get_path($product, q(fasta), $self->repository());
289+
if ($ref_fasta) {
290+
$init_hash{'reference_fasta'} = $ref_fasta;
328291
}
329-
330-
my $class = $self->_qc_module_name();
331-
my $check_obj;
332-
try {
333-
$check_obj = $class->new(\%init_hash);
334-
} catch {
335-
delete $init_hash{'lims'};
336-
$check_obj = $class->new(\%init_hash);
337-
};
338-
339-
$can_run = $check_obj->can_run();
340292
}
341293

342-
return $can_run;
294+
my $class = $self->_qc_module_name();
295+
my $check_obj;
296+
try {
297+
$check_obj = $class->new(\%init_hash);
298+
} catch {
299+
delete $init_hash{'lims'};
300+
$check_obj = $class->new(\%init_hash); # Allow to fail at this stage.
301+
};
302+
303+
return $check_obj->can_run();
343304
}
344305

345306
__PACKAGE__->meta->make_immutable;

0 commit comments

Comments
 (0)