BUG: Augur translate falsely claims BCBio.GFF not found when no features file passed

[corneliusroemer]

Current Behavior

When running the following command:

augur translate             --tree results/tree.nwk             --ancestral-sequences results/nt_muts.json \
          --reference-sequence data/JX869059_genemap.gff             --output-node-data results/aa_muts.json    

I get the error:

Validating schema of 'results/nt_muts.json'...
ERROR: Package BCBio.GFF not found! Please install using 'pip install bcbio-gff' before re-running.
Traceback (most recent call last):
  File "/opt/homebrew/Caskroom/miniforge/base/envs/nextstrain/lib/python3.10/site-packages/augur/__init__.py", line 67, in run
    return args.__command__.run(args)
  File "/opt/homebrew/Caskroom/miniforge/base/envs/nextstrain/lib/python3.10/site-packages/augur/translate.py", line 354, in run
    print("Read in {} features from reference sequence file".format(len(features)))
TypeError: object of type 'NoneType' has no len()


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>

I confirmed that bcbio-gff is installed in my conda environment as (0.6.9 pyh5e36f6f_0 bioconda).

Expected behavior

The error seems to be that len is calculated of a features variable which does not exist. This should have been validated earlier or caught appropriately. Reporting that BCBio.GFF isn't found is definitely incorrect.

Also, it's maybe not ideal that we suggest to pip install - when we usually recommend conda/mamba nowadays. I couldn't find where the pip install line was coming from in a quick search.

Your environment: if browsing Nextstrain online

• Operating system:

Your environment: if running Nextstrain locally

• Operating system: macOS M1
• Version (e.g. auspice 2.7.0): augur 21.0.0
• x86 installed via conda

[corneliusroemer]

Apparently we don't raise here, just returning none and printing an error message, not sure why we say "Error" and then not fail:

augur/augur/utils.py

Line 158 in 53fed2d

print("ERROR: Package BCBio.GFF not found! Please install using \'pip install bcbio-gff\' before re-running.")

This is what I get if I try to import BCBio in the same conda environment manually:

>>> from BCBio import GFF 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/homebrew/Caskroom/miniforge/base/envs/nextstrain/lib/python3.10/site-packages/BCBio/GFF/__init__.py", line 3, in <module>
    from BCBio.GFF.GFFParser import GFFParser, DiscoGFFParser, GFFExaminer, parse, parse_simple
  File "/opt/homebrew/Caskroom/miniforge/base/envs/nextstrain/lib/python3.10/site-packages/BCBio/GFF/GFFParser.py", line 34, in <module>
    from Bio.Seq import UnknownSeq
ImportError: cannot import name 'UnknownSeq' from 'Bio.Seq' (/opt/homebrew/Caskroom/miniforge/base/envs/nextstrain/lib/python3.10/site-packages/Bio/Seq.py)

[corneliusroemer]

Aha, I have biopython 1.81 installed, which was released fairly recently, apparently 5 days ago? So we may need to restrict Augur to Biopython <=1.81 for now 😬 Seems like Biopython deprecated UnknownSeq without bumping a major version

And indeed, 1.81 appeared on Biopython just 1:15hr ago.

[corneliusroemer]

BCBio.GFF is only used in one place and by augur translate only.

[corneliusroemer]

Bug report from 4d ago in BCBio: chapmanb/bcbb#136

[corneliusroemer]

Now also visible in CI: https://github.com/nextstrain/augur/actions/runs/4205049112/jobs/7296575834#step:9:79

Maybe a case for running CI at least daily to automatically notice if a new package version causes issues? In this case, I spotted it quickly because I happened to manually update my environment and run into the bug in the wild, but would be good not to depend on manual updates or user reports.

Given we install packages from Pypi preferentially, we should have been able to spot this bug already 4 days ago - since Pypi was about 4d ahead of conda-forge's 1.81 release.

[corneliusroemer]

@huddlej I tagged the PR #1152 as only partially resolving this issue as we are still not really handling import errors properly. We shouldn't return None but bubble up the error as an Augur exception, or whatever we've been using. @victorlin has been in charge of properly raising errors that we know can happen.