Ability to set a path for NovaSeqX samplesheet #795
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mgcam
force-pushed
the
novaseqx_ssheet_path
branch
from
8336a6b to
d352987
Compare
mgcam
force-pushed
the
novaseqx_ssheet_path
branch
from
d352987 to
a5a1ce3
Compare
mgcam
changed the title
mgcam
force-pushed
the
novaseqx_ssheet_path
branch
from
e37aa34 to
86531f4
Compare
Created a parent class for two existing samplesheet generators. Moved common methods to this class. Through this inheritance the NovaSeqX samplesheet generator gets a standard method for setting a configurable path for a samplesheet.
mgcam
force-pushed
the
novaseqx_ssheet_path
branch
from
86531f4 to
e8e50f2
Compare
jmtcsngr
reviewed
Dec 20, 2023
lib/npg/samplesheet/base.pm
Outdated
|
|
||
| =item npg_tracking::Schema | ||
|
|
||
| =item st::api::lims |
There was a problem hiding this comment.
Suggested change
| =item st::api::lims | |
| =item st::api::lims |
|
|
||
| =head2 batch_id | ||
| my $date = DateTime->now()->strftime('%y%m%d'); # 230602 for 2 June 2023 |
There was a problem hiding this comment.
Suggested change
| my $date = DateTime->now()->strftime('%y%m%d'); # 230602 for 2 June 2023 | |
| my $date = DateTime->now()->strftime('%y%m%d'); # 230602 for 2 June 2023 |
| my $dir = $self->samplesheet_path(); | ||
| my $path = $self->file_name; | ||
| if ($dir) { | ||
| $dir =~ s{/\Z}{}xms; # Trim trailing slash if present. |
There was a problem hiding this comment.
Should this regex be one or more '/' at end of line?
| if ($self->has_id_run()) { | ||
| if ($self->id_run()) { | ||
| if ($self->run->instrument_format()->model() !~ /NovaSeqX/smx) { | ||
| croak 'Instrument model is not NovaSeq X Series'; |
There was a problem hiding this comment.
Could this be something like 'Instrument model is not NovaSeq X Series in tracking' Or 'Instrument model must be NovaSeqX series but is ' . $self->run->instrument_format()->model() . ' in tracking database'?
Ah but then is also changing test. Which may be bad for just refactoring. Well, only if you think it is worth.
lib/npg/samplesheet/base.pm
Outdated
| 'lazy_build' => 1, | ||
| ); | ||
| sub _build_samplesheet_path { | ||
| if($ENV{dev} and not $ENV{dev}=~/live/smix){ |
There was a problem hiding this comment.
Probably this is not needed since paths can be set by whoever executes.
An option to read the value from the configuration file is not used.
mgcam
force-pushed
the
novaseqx_ssheet_path
branch
from
e8e50f2 to
5aa5005
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Built on top of #794
Created a parent class for two existing samplesheet
generators. Moved common methods to this class.
Through this inheritance the NovaSeqX samplesheet
generator gets a standard method for setting a path for the
samplesheet.
An option to set the path from a value in the configuration file
is dropped - it was not used for years.