Skip to content

Commit

Permalink
Merge pull request #900 from wtsi-npg/devel
Browse files Browse the repository at this point in the history 
pull from devel to master to create release 74.0.1
  • Loading branch information
@jmtcsngr
jmtcsngr authored Dec 17, 2024
2 parents f9f614f + cf588d5 commit 3d77241
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 77 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/run-tests.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ jobs:
sudo apt-get update
# https://github.com/actions/runner-images/issues/2139
sudo apt-get remove -y nginx libgd3
sudo apt-get install -y libgd-dev uuid-dev libgd-text-perl
# libdevel-patchperl-perl is needed for perlbrew
sudo apt-get install -y libgd-dev uuid-dev libgd-text-perl libdevel-patchperl-perl
- name: "Install Miniforge"
run: |
Expand Down
10 changes: 10 additions & 0 deletions Changes
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
LIST OF CHANGES FOR NPG-QC PACKAGE

release 74.0.1 (2024-12-17)
- Fix perlbrew installation by installing libdevel-patchperl-perl in runner
- Fixes/updates for lane-level RoboQC lane-level assessment:
- Excluded study-specific lane-lane assessment in npg_qc::autoqc::check::review
This helps to avoid uncertainty in evaluating lanes where samples belong to
different studies at a price of not being able to perform study-specific
evaluation of one-library lanes.
- Updated the expression evaluator to allow for multiple types of
result objects to be used in a single expression.

release 74.0.0 (2024-12-02)
- npg_qc::autoqc::qc_store - changed loading of ORM classes into memory
to on demand at run time.
Expand Down
169 changes: 106 additions & 63 deletions lib/npg_qc/autoqc/checks/review.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,36 @@ our $VERSION = '0';
Readonly::Scalar my $CONJUNCTION_OP => q[and];
Readonly::Scalar my $DISJUNCTION_OP => q[or];

Readonly::Scalar my $ROBO_KEY => q[robo_qc];
Readonly::Scalar my $CRITERIA_KEY => q[criteria];
Readonly::Scalar my $QC_TYPE_KEY => q[qc_type];
Readonly::Scalar my $ROBO_KEY => q[robo_qc];
Readonly::Scalar my $CRITERIA_KEY => q[criteria];
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 $ACCEPTANCE_CRITERIA_KEY => q[acceptance_criteria];

Readonly::Scalar my $QC_TYPE_LIB => q[mqc];
Readonly::Scalar my $QC_TYPE_SEQ => q[mqc_seq];

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

Readonly::Scalar my $CLASS_NAME_SPEC_DELIM => q[:];
Readonly::Scalar my $CLASS_NAME_SPEC_RE =>
qr/\A(?:\W+)? (\w+) (?: $CLASS_NAME_SPEC_DELIM (\w+))?[.]/xms;
Readonly::Scalar my $CLASS_NAME_SPEC_RE =>
qr{
# Examples:
# "generic:ncov2019_artic_nf.doc->{meta}->{'num_input_reads'} > 10000"
# "tag_metrics.all_reads && (qX_yield.yield1_q30/(tag_metrics.all_reads * 302) > 78)"
(?:\W+)? # optional non-word characters
( # start of capture group
[[:alpha:]_]+ # result class name
(?: $CLASS_NAME_SPEC_DELIM \w+)? # an optional spec for a generic result
# class prepended by a delimeter
) # end of capture group
[.] # ends with a dot
}xms;

## no critic (Documentation::RequirePodAtEnd)

Expand Down Expand Up @@ -74,7 +85,8 @@ outcomes.
The C<robo> section of the product configuration file sits either within
the configuration for a particular study or in the C<default> section, or
in both locations. For a given entity a study-specific RoboQC definition
takes precedence over the default one.
takes precedence over the default one. For lanes only the C<default> section
of the product configuration file is examined.
Evaluation criteria for samples vary depending on the sequencing instrument
and flowcell type, library type, sample reference, etc. Each of the
Expand Down Expand Up @@ -487,14 +499,24 @@ has '_robo_config' => (
sub _build__robo_config {
my $self = shift;

my $strict = 1; # Parse study section only, ignore the default section.
my $config = $self->study_config($self->lims(), $strict);
if ($config) {
$config = $config->{$ROBO_KEY};
my $config;

# Do not examine study-specific section for lane entities, meaning that
# one-sample lanes, pools or not, cannot be evaluated according to the
# study criteria of their sample. Helps to avoid uncertainty of dealing
# with pool's samples belonging to different studies.
if (!$self->lims->is_lane) {
my $strict = 1; # Parse study section only, ignore the 'default' section.
$config = $self->study_config($self->lims(), $strict);
if ($config) {
$config = $config->{$ROBO_KEY};
}
if (!$config) {
carp 'Study-specific RoboQC config not found for ' . $self->_entity_desc;
}
}

if (!$config) {
carp 'Study-specific RoboQC config not found for ' . $self->_entity_desc;
if (!$config) { # Look at the 'default' section of the configuration file.
$config = $self->default_study_config()->{$ROBO_KEY};
}

Expand Down Expand Up @@ -652,7 +674,7 @@ sub _build__result_class_names {
my $self = shift;

my @class_names = uniq sort
map { (_class_name_from_expression($_))[0] }
map { _class_names_from_expression($_) }
map { @{$_} } # dereference the array
values %{$self->_criteria()};

Expand Down Expand Up @@ -721,11 +743,26 @@ sub _build__results {
return \%h;
}

sub _class_name_from_expression {
sub _class_names_with_spec_from_expression {
my $e = shift;
# Note 'g' flag at the end of the regexp. We will get a list of matches.
# The list might contain multiple class names for the same class. Depending
# on the expression, the return value contains one or more class names.
my @extended_class_names = $e =~ /$CLASS_NAME_SPEC_RE/xmsg;
@extended_class_names = uniq @extended_class_names; # Remove repetitions.
@extended_class_names or croak "Failed to infer class names from $e";
return @extended_class_names;
}

sub _class_names_from_expression {
my $e = shift;
my ($class_name, $spec) = $e =~ $CLASS_NAME_SPEC_RE;
$class_name or croak "Failed to infer class name from $e";
return ($class_name, $spec);
my @names = ();
foreach my $name (_class_names_with_spec_from_expression($e)) {
($name) = $name =~ /\A(\w+)/smx; # Remove the spec part if present.
push @names, $name;
}
@names = uniq @names;
return @names;
}

#####
Expand Down Expand Up @@ -785,62 +822,68 @@ sub _apply_operator {
return $outcome ? 1 : 0;
}

sub _pp_name2spec {
# To get a match with what would have been used in the robo config,
# replace all 'non-word' characters.
my $pp_name = shift;
$pp_name =~ s/\W/_/gsmx;
return $pp_name;
};

sub _get_evaluation_result {
my ($perl_e, $e, $results) = @_;

use warnings FATAL => 'uninitialized'; # Force an error when operations on
# undefined values are attempted.
##no critic (BuiltinFunctions::ProhibitStringyEval)
my $o = eval $perl_e; # Evaluate Perl expression
##use critic
if ($EVAL_ERROR) {
my $err = $EVAL_ERROR;
croak "Error evaluating expression '$perl_e' derived from '$e': $err";
}

return $o ? 1 : 0;
}

#####
# Evaluates a single expression in the context of available autoqc results.
# If runs successfully, returns 0 or 1, otherwise throws an error.
#
sub _evaluate_expression {
my ($self, $e) = @_;

my ($class_name, $spec) = _class_name_from_expression($e);
my $obj_a = $self->_results->{$class_name};
# We should not get this far with an error in the configuration
# file, but just in case...
$obj_a and @{$obj_a} or croak "No autoqc result for evaluation of '$e'";

if ($spec) {
my $pp_name2spec = sub { # To get a match with what would have been
my $pp_name = shift; # used in the robo config, replace all
$pp_name =~ s/\W/_/gsmx; # 'non-word' characters.
return $pp_name;
};
# Now have to choose one result. If the object is not an instance of
# the generic autoqc result class, there will be an error at this point.
# Making the code less specific is not worth the effort at this point.
my @on_spec = grep { $pp_name2spec->($_->pp_name) eq $spec } @{$obj_a};
@on_spec or croak "No autoqc $class_name result for $spec";
$obj_a = \@on_spec;
}

(@{$obj_a} == 1) or croak "Multiple autoqc results for evaluation of '$e'";
my $obj = $obj_a->[0];

$obj or croak "No autoqc result for evaluation of '$e'";

# Prepare the expression from the robo config for evaluation.
my $placeholder = $class_name;
$spec and ($placeholder .= $CLASS_NAME_SPEC_DELIM . $spec);
my $replacement = q[$] . q[result->];
my @names = _class_names_with_spec_from_expression($e);
my $results = {};
my $perl_e = $e;
$perl_e =~ s/$placeholder[.]/$replacement/xmsg;

my $evaluator = sub { # Evaluation function
my $result = shift;
# Force an error when operations on undefined values are
# are attempted.
use warnings FATAL => 'uninitialized';
##no critic (BuiltinFunctions::ProhibitStringyEval)
my $o = eval $perl_e; # Evaluate Perl string expression
##use critic
if ($EVAL_ERROR) {
my $err = $EVAL_ERROR;
croak "Error evaluating expression '$perl_e' derived from '$e': $err";
# Prepare the expression from the robo config for evaluation.
foreach my $name (@names) {
my ($class_name, $spec) = split /$CLASS_NAME_SPEC_DELIM/smx, $name;
my $obj_a = $self->_results->{$class_name};
# We should not get this far with an error in the configuration
# file, but just in case...
$obj_a and @{$obj_a} or croak "No $class_name autoqc result for evaluation of '$e'";
if ($spec) {
# Now have to choose one result. If the object is not an instance of
# the generic autoqc result class, there will be an error at this point.
# Making the code less specific is not worth the effort at this point.
my @on_spec = grep { _pp_name2spec($_->pp_name) eq $spec } @{$obj_a};
@on_spec or croak "No $name autoqc result for evaluation of '$e'";
$obj_a = \@on_spec;
}
return $o ? 1 : 0;
};
(@{$obj_a} == 1) or croak "Multiple $name autoqc results for evaluation of '$e'";
my $obj = $obj_a->[0];
$obj or croak "No $class_name autoqc result for evaluation of '$e'";
$results->{$name} = $obj;

## no critic (ValuesAndExpressions::RequireInterpolationOfMetachars)
my $replacement = q[$results->{'] . $name . q['}->];
## use critic
$perl_e =~ s/$name[.]/$replacement/xmsg;
}

# Evaluate and return the outcome.
return $evaluator->($obj);
return _get_evaluation_result($perl_e, $e, $results);
}

__PACKAGE__->meta->make_immutable();
Expand Down
6 changes: 3 additions & 3 deletions scripts/install_miniforge.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

set -ex

MINIFORGE_VERSION="24.9.0-0"
MINIFORGE_SHA256="77fb505f6266ffa1b5d59604cf6ba25948627e908928cbff148813957b1c28af"
MINIFORGE_VERSION="24.9.2-0"
MINIFORGE_SHA256="ca8c544254c40ae5192eb7db4e133ff4eb9f942a1fec737dba8205ac3f626322"

CONDA_HOME=${CONDA_HOME:="$HOME/conda"}
export CONDA_HOME
Expand All @@ -27,7 +27,7 @@ channels:
- conda-forge
EOF

curl -sSL "https://github.com/conda-forge/miniforge/releases/download/${MINIFORGE_VERSION}/Mambaforge-${MINIFORGE_VERSION}-Linux-x86_64.sh" -o ./miniforge.sh
curl -sSL "https://github.com/conda-forge/miniforge/releases/download/${MINIFORGE_VERSION}/Miniforge3-${MINIFORGE_VERSION}-Linux-x86_64.sh" -o ./miniforge.sh
sha256sum ./miniforge.sh | grep "$MINIFORGE_SHA256"
/bin/bash ./miniforge.sh -b -p "$CONDA_HOME"
rm ./miniforge.sh
Loading

0 comments on commit 3d77241

Please sign in to comment.