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

SN DSA Release #277

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

SN DSA Release #277

wants to merge 19 commits into from

Conversation

sarahgonicholson
Copy link
Contributor

@sarahgonicholson sarahgonicholson commented Oct 16, 2024

In commands/release-file.py and commands/create-annotated-filenames.py:

  • Assay and sequencer codes value set to XX for DSA fasta files and chain files
  • For Supplementary Files, use title instead of code for software part of annotated filename

@sarahgonicholson sarahgonicholson marked this pull request as ready for review October 16, 2024 20:17
@alexander-veit
Copy link
Contributor

The tests are failing. I think it's related to your latest change

Copy link
Contributor

@alexander-veit alexander-veit left a comment

Choose a reason for hiding this comment

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

Generally looks good. Some small clarifications needed. Also test issue must be adressed.

@@ -750,14 +752,20 @@ def get_sex_abbreviation(sex: str) -> str:


def get_sequencing_and_assay_codes(
file: List[Dict[str, Any]],
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct? Is it a list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixing!

@@ -816,7 +824,9 @@ def get_analysis(
software_and_versions, reference_genome_code
)
if file_format_utils.is_chain_file(file_extension):
value = f"{value}{ANALYSIS_INFO_SEPARATOR}{get_chain_file_value(file)}"
value = ANALYSIS_INFO_SEPARATOR.join([value,get_chain_file_value(file)]) if value else get_chain_file_value(file)
if file_format_utils.is_fasta_file(file_extension):
Copy link
Contributor

Choose a reason for hiding this comment

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

Not quite sure I understand. You are only testing for fasta file here. Do you always want to appenddsa in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point. I'll add a specification for only DSA fasta files

file_constants.DATASET: dataset,
file_constants.ACCESS_STATUS: access_status,
file_constants.ANNOTATED_FILENAME: annotated_filename_info.filename,
}

if not supp_file_utils.is_reference_conversion(self.file) and not supp_file_utils.is_reference_genome(self.file):
Copy link
Contributor

Choose a reason for hiding this comment

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

You are making this check already in get_file_sets_from_file. Should we just test here if file_set_accessions is empty?

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