Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved samplesheet generation for NovaSeqX. #794

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

mgcam
Copy link
Member

@mgcam mgcam commented Dec 15, 2023

Commented out code deleted.
Attributes and methods are ordered more logically. Some private accesors are converted to public.
Documentation improved.
Added index_read_length and read_length attributes so that these values can be set by the caller.

Commented out code deleted.
Attributes and methods are ordered more logically.
Some private accesors are converted to public.
Documentation improved.
Added index_read_length and read_length attributes so that
these values can be set by the caller.
@mgcam mgcam force-pushed the novaseqx_ssheet_generator_refactor branch from 2f9ced9 to 7b3e06e Compare December 15, 2023 14:30
Copy link
Member

@jmtcsngr jmtcsngr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only suggestions and a couple of questions.


C<dragen_max_number_of_configs> -
DRAGEN analysis can deal with a limited number of distinct
configurations. Set this attribute if processing off-board.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think file_name may be missing from the documentation?

- Must be same number of fields (delimited by semicolon) as sequencing and
indexing reads specified in RunInfo.xml or Reads section.

- Indexing reads are specified with I, sequencing reads are specified with
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Indexing reads are specified with I, sequencing reads are specified with
- Indexing reads are specified with 'I', sequencing reads are specified with

indexing reads specified in RunInfo.xml or Reads section.

- Indexing reads are specified with I, sequencing reads are specified with
Y, UMI cycles are specified with U, and trimmed reads are specified with N.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Y, UMI cycles are specified with U, and trimmed reads are specified with N.
'Y', UMI cycles are specified with 'U', and trimmed reads are specified with 'N'.

requirements:

- Must be same number of fields (delimited by semicolon) as sequencing and
indexing reads specified in RunInfo.xml or Reads section.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
indexing reads specified in RunInfo.xml or Reads section.
indexing reads specified in RunInfo.xml or 'Reads' section.

- The number of cycles specified for each read must equal the number of cycles
specified for that read in the RunInfo.xml file.

- Only one Y or I sequence can be specified per read.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Only one Y or I sequence can be specified per read.
- Only one 'Y' or 'I' sequence can be specified per read.

'lazy_build' => 1,
'required' => 0,
'documentation' => 'An array containing the length of the first and the ' .
'second (if applicable) indexing read..',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'second (if applicable) indexing read..',
'second (if applicable) indexing read.',


=cut

has 'index_read_length' => (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this meant to be used as an override? Should we care if only one is provided at executing time but there are two lengths in LIMS?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is when we want to do something semi-manually. Prior to the start of the run we only have LIMS information. If we want to run DRAGEN off-board, then we will find out what happenned from RunInfo.xml As we know only two well, all sorts of discrepancies are possible.

@jmtcsngr jmtcsngr merged commit dbf9dd5 into wtsi-npg:devel Dec 19, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants