EVA-3563 remove the option of providing csv mapping through command line #37
Conversation
| @@ -53,6 +53,7 @@ Analysis: | |||
| Experiment Type: experimentType | |||
| Reference: referenceGenome | |||
| optional: | |||
| Assembly Fasta: assemblyFasta | |||
There was a problem hiding this comment.
I would suggest to make the name explicit.
| Assembly Fasta: assemblyFasta | |
| Assembly Fasta Path: assemblyFasta |
eva_sub_cli/main.py
Outdated
| if not os.path.exists(os.path.abspath(metadata_file)): | ||
| raise Exception(f'The provided metadata file {metadata_file} does not exist') |
There was a problem hiding this comment.
Can we create a specific exception for command line error that can be caught in the main binary so that a clean error message is provided and not a code exception that looks like a crash ?
There was a problem hiding this comment.
Updated to FileNotFoundError
There was a problem hiding this comment.
I don't think Assembly Fasta should be in bold since it's optional.
eva_sub_cli/etc/eva_schema.json
Outdated
| }, | ||
| "assemblyFasta": { | ||
| "type": "string", | ||
| "description": "The assembly in fasta format that was used to derive the VCF" |
There was a problem hiding this comment.
| "description": "The assembly in fasta format that was used to derive the VCF" | |
| "description": "The path to the assembly in fasta format that was used to create the VCF. (This is a local path)" |
eva_sub_cli/etc/eva_schema.json
Outdated
| }, | ||
| "assemblyReport": { | ||
| "type": "string", | ||
| "description": "The assembly report associated with the assembly as found in NCBI assemblies" |
There was a problem hiding this comment.
| "description": "The assembly report associated with the assembly as found in NCBI assemblies" | |
| "description": "The path to the assembly report associated with the assembly as found in NCBI assemblies. (This is a local path)" |
| @@ -58,8 +53,6 @@ def get_version(): | |||
| vcf_group.add_argument('--vcf_files', nargs='+', help="One or several vcf files to validate") | |||
| vcf_group.add_argument('--assembly_fasta', | |||
| help="The fasta file containing the reference genome from which the variants were derived") | |||
| vcf_group.add_argument("--vcf_files_mapping", | |||
| help="csv file with the mappings for vcf files, fasta and assembly report") | |||
There was a problem hiding this comment.
Can you update the readme as well? It's just easier to keep the documentation up-to-date when we do it in parallel with the code updates.
eva_sub_cli/etc/eva_schema.json
Outdated
| @@ -165,6 +165,14 @@ | |||
| "referenceGenome": { | |||
| "type": "string", | |||
| "description": "Reference against which the analysis was performed. ENA or NCBI accession (starting with GCA) accepted." | |||
| }, | |||
| "assemblyFasta": { | |||
There was a problem hiding this comment.
In the spreadsheet we request "Reference" and state it can be "GCA accession for assemblies or associated INSDC accession for sequences", and then ask for "Assembly Fasta Path" which sounds like it should be for an assembly only.
Meanwhile here in the JSON we request "referenceGenome" and say it has to start with GCA.
I think we should try to be consistent with our terminology (and also whether we state we accept single INSDC sequences or not...)
There was a problem hiding this comment.
Excel does this dumb thing where it opens the workbook on whatever tab it was last saved, which is a bit disorienting if it's not the "Please read first" tab.... doesn't really matter now since it's just us, but just to keep in mind once we have external users.
…nstead use metadata files
review comments - indentation fix Co-authored-by: Timothee Cezard <tcezard@ebi.ac.uk>
nitin-ebi
force-pushed
the
remove_mapping_file_option
branch
from
70fba2a
to
3b54021
Compare
No description provided.