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

ENH(translate): Better input arg validation for #1181

Closed
corneliusroemer opened this issue Mar 14, 2023 · 1 comment
Closed

ENH(translate): Better input arg validation for #1181

corneliusroemer opened this issue Mar 14, 2023 · 1 comment

Comments

@corneliusroemer
Copy link
Member

Context

I just wanted to test a minimal example of augur translate to see whether the bcbio-gff issue #1151 is still fixed if we upgrade bcbio-gff to 0.7.0 in PR #1178

Description

I was surprised to find that the augur translate argument validation is basically non-existent. This results in bad usability, the error messages are not helpful.

We should figure out what the various required args are and report to the user when an arg is missing rather than throwing an error somewhere in a function that assumes that the input is present.

Example session

I tried to build a command up from nothing and none of the errors were pointing me in the right direction, it seems all args but --reference-sequence are optional and there's no further testing of what args were passed until the args are used deep in the program:

augur translate                                                                                                                                                                                                              
usage: augur translate [-h] [--tree TREE] [--ancestral-sequences ANCESTRAL_SEQUENCES] --reference-sequence REFERENCE_SEQUENCE [--genes GENES [GENES ...]] [--output-node-data OUTPUT_NODE_DATA] [--alignment-output ALIGNMENT_OUTPUT] [--vcf-reference-output VCF_REFERENCE_OUTPUT]
                       [--vcf-reference VCF_REFERENCE]
augur translate: error: the following arguments are required: --reference-sequence

augur on  unpin-biopython-pin-bcbio-gff [$?] via 🐍 v3.8.13 via 🅒 augur-dev took 19s 
❌2 ❯ z sars                     

nextclade_data_workflows/sars-cov-2 on  master [$] via 🅒 augur-dev augur translate --reference-sequence defaults/reference_seq.gb
Traceback (most recent call last):
  File "/Users/corneliusromer/code/augur/augur/__init__.py", line 67, in run
    return args.__command__.run(args)
  File "/Users/corneliusromer/code/augur/augur/translate.py", line 334, in run
    tree = Phylo.read(args.tree, 'newick')
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/site-packages/Bio/Phylo/_io.py", line 60, in read
    tree = next(tree_gen)
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/site-packages/Bio/Phylo/_io.py", line 49, in parse
    yield from getattr(supported_formats[format], "parse")(fp, **kwargs)
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/site-packages/Bio/Phylo/NewickIO.py", line 52, in parse
    return Parser(handle).parse(**kwargs)
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/site-packages/Bio/Phylo/NewickIO.py", line 104, in __init__
    if handle.read(0) != "":
AttributeError: 'NoneType' object has no attribute 'read'


An error occurred (see above) that has not been properly handled by Augur.
To report this, please open a new issue including the original command and the error above:
    <https://github.com/nextstrain/augur/issues/new/choose>


nextclade_data_workflows/sars-cov-2 on  master [$] via 🅒 augur-dev 
❌2 ❯ augur translate --reference-sequence defaults/reference_seq.gb --tree builds/21L/tree.nwk
Traceback (most recent call last):
  File "/Users/corneliusromer/code/augur/augur/__init__.py", line 67, in run
    return args.__command__.run(args)
  File "/Users/corneliusromer/code/augur/augur/translate.py", line 344, in run
    if any([args.ancestral_sequences.lower().endswith(x) for x in ['.vcf', '.vcf.gz']]):
  File "/Users/corneliusromer/code/augur/augur/translate.py", line 344, in <listcomp>
    if any([args.ancestral_sequences.lower().endswith(x) for x in ['.vcf', '.vcf.gz']]):
AttributeError: 'NoneType' object has no attribute 'lower'


An error occurred (see above) that has not been properly handled by Augur.
To report this, please open a new issue including the original command and the error above:
    <https://github.com/nextstrain/augur/issues/new/choose>


nextclade_data_workflows/sars-cov-2 on  master [$] via 🅒 augur-dev 
❌2 ❯ augur translate --reference-sequence defaults/reference_seq.gb --tree builds/21L/tree.nwk --ancestral-sequences builds/21L/masked.fasta
Traceback (most recent call last):
  File "/Users/corneliusromer/code/augur/augur/__init__.py", line 67, in run
    return args.__command__.run(args)
  File "/Users/corneliusromer/code/augur/augur/translate.py", line 353, in run
    node_data = read_node_data(args.ancestral_sequences, args.tree)
  File "/Users/corneliusromer/code/augur/augur/utils.py", line 93, in read_node_data
    return NodeDataReader(fnames, tree, validation_mode).read()
  File "/Users/corneliusromer/code/augur/augur/util_support/node_data_reader.py", line 30, in read
    node_data = self.build_node_data()
  File "/Users/corneliusromer/code/augur/augur/util_support/node_data_reader.py", line 39, in build_node_data
    for node_data_file in self.node_data_files:
  File "/Users/corneliusromer/code/augur/augur/util_support/node_data_reader.py", line 55, in <genexpr>
    return (NodeDataFile(fname, validation_mode = self.validation_mode) for fname in self.filenames)
  File "/Users/corneliusromer/code/augur/augur/util_support/node_data_file.py", line 20, in __init__
    self.attrs = json.load(jfile)
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/json/__init__.py", line 293, in load
    return loads(fp.read(),
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/json/__init__.py", line 357, in loads
    return _default_decoder.decode(s)
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)


An error occurred (see above) that has not been properly handled by Augur.
To report this, please open a new issue including the original command and the error above:
    <https://github.com/nextstrain/augur/issues/new/choose>


nextclade_data_workflows/sars-cov-2 on  master [$] via 🅒 augur-dev 
❌2 ❯ augur translate --reference-sequence defaults/reference_seq.gb --tree builds/21L/tree.nwk --ancestral-sequences builds/21L/masked.fasta --output-node-data test.json
Traceback (most recent call last):
  File "/Users/corneliusromer/code/augur/augur/__init__.py", line 67, in run
    return args.__command__.run(args)
  File "/Users/corneliusromer/code/augur/augur/translate.py", line 353, in run
    node_data = read_node_data(args.ancestral_sequences, args.tree)
  File "/Users/corneliusromer/code/augur/augur/utils.py", line 93, in read_node_data
    return NodeDataReader(fnames, tree, validation_mode).read()
  File "/Users/corneliusromer/code/augur/augur/util_support/node_data_reader.py", line 30, in read
    node_data = self.build_node_data()
  File "/Users/corneliusromer/code/augur/augur/util_support/node_data_reader.py", line 39, in build_node_data
    for node_data_file in self.node_data_files:
  File "/Users/corneliusromer/code/augur/augur/util_support/node_data_reader.py", line 55, in <genexpr>
    return (NodeDataFile(fname, validation_mode = self.validation_mode) for fname in self.filenames)
  File "/Users/corneliusromer/code/augur/augur/util_support/node_data_file.py", line 20, in __init__
    self.attrs = json.load(jfile)
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/json/__init__.py", line 293, in load
    return loads(fp.read(),
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/json/__init__.py", line 357, in loads
    return _default_decoder.decode(s)
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/opt/homebrew/Caskroom/miniforge/base/envs/augur-dev/lib/python3.8/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)


An error occurred (see above) that has not been properly handled by Augur.
To report this, please open a new issue including the original command and the error above:
    <https://github.com/nextstrain/augur/issues/new/choose>
@jameshadfield
Copy link
Member

After #1348 and #1355 this issue is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

2 participants