-
Notifications
You must be signed in to change notification settings - Fork 0
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
SN DSA Release #277
Conversation
The tests are failing. I think it's related to your latest change |
There was a problem hiding this 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]], |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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): |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
src/encoded/commands/release_file.py
Outdated
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): |
There was a problem hiding this comment.
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?
In
commands/release-file.py
andcommands/create-annotated-filenames.py
:haplotype
,target_assembly
, andsource_assembly
properties to create annotated filenames for chain and fasta files