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

Merged
merged 30 commits into from
Dec 20, 2024
Merged

SN DSA Release #277

merged 30 commits into from
Dec 20, 2024

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 haplotype, target_assembly, and source_assembly properties to create annotated filenames for chain and fasta files

@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?

@sarahgonicholson sarahgonicholson merged commit bbf512b into main Dec 20, 2024
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