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.

Sign up for GitHub

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

Jump to bottom

Error in translate.py from vcf #591

Closed
andreott opened this issue Jul 9, 2020 · 3 comments
Closed

Error in translate.py from vcf #591

andreott opened this issue Jul 9, 2020 · 3 comments
Labels
bug Something isn't working easy problem Requires less work than most issues priority: high To be resolved before other issues

Comments

@andreott
Copy link

andreott commented Jul 9, 2020

When using translate for vcf an error comes up for the sequence length not being a multiple of 3. However this test is performed for the source feature. Probably the if feat.type != 'source': should be placed at the outer loop level, e.g.

for fname, feat in features.items():
    if feat.type != 'source':
        if is_vcf:
            trans = translate_vcf_feature(sequences, ref, feat)
            if trans:
                translations[fname] = trans
            else:
                deleted.append(fname)
        else:
            translations[fname] = translate_feature(sequences, feat)

instead of

augur/augur/translate.py

Lines 359 to 368 in 3dcf8a1

for fname, feat in features.items():
if is_vcf:
trans = translate_vcf_feature(sequences, ref, feat)
if trans:
translations[fname] = trans
else:
deleted.append(fname)
else:
if feat.type != 'source':
translations[fname] = translate_feature(sequences, feat)

@huddlej huddlej added bug Something isn't working easy problem Requires less work than most issues priority: high To be resolved before other issues labels Aug 14, 2020
@andreott andreott mentioned this issue Dec 8, 2022
Closed
andreott added a commit to andreott/augur that referenced this issue Dec 8, 2022
@andreott
[fix]
c4ab7e3 
Fixes nextstrain#591 in translate.py by perfoming the check for feature type source for vcf and alignment mode.

Fixes [1108] by moving the warnung about padding outside of translate_vcf_feature. Padding will happen in both cases - not only vcf - and the feature name is also available at this point
andreott added a commit to andreott/augur that referenced this issue Dec 8, 2022
@andreott
Fixes nextstrain#591 in translate.py by perfoming the check for featu…
fb33d63 
…re type source for vcf and alignment mode.

Fixes nextstrain#1108 by moving the warning about padding outside of translate_vcf_feature where the feature name is also available and does not need to be passed on to translate_vcf_feature
@andreott andreott mentioned this issue Dec 8, 2022
Closed
1 task
@TheBready
Copy link

TheBready commented Nov 21, 2023 •
edited
Loading

Hey @andreott, is this issue solved and the fix merged?

@jameshadfield
Copy link
Member

It seems this issue and the corresponding PR didn't get attention from us - apologies.

I'm currently doing some work on general VCF parsing and how this information flows through augur ancestral and augur translate, and part of this will address this issue. It should be up in the coming week or so.

jameshadfield added a commit that referenced this issue Dec 4, 2023
@jameshadfield
[translate] skip parsing source (nuc) as a feature
525f7e0 
This check is already in place for non-VCF inputs, and my guess is it
was omitted here as the TB pipeline's GFF file didn't include a 'source'
annotation. I don't think 'source' is actually a valid GFF ID and I
suspect we've just been applying the INSDC/GenBank term to GFF files,
but it is one of the two fields parsed by `load_features` and there are
GFF files in Nextstrain build pipelines which use it. Modifying the
underlying `load_features` would be a better solution, but that's a
bigger project for another day.

We additionally update the error message to use the same feature name we
export.

Closes #591
Supersedes #1109
@jameshadfield jameshadfield mentioned this issue Dec 4, 2023
Merged
2 tasks
jameshadfield added a commit that referenced this issue Dec 4, 2023
@jameshadfield
[translate] skip parsing source (nuc) as a feature
2c201bf 
This check is already in place for non-VCF inputs, and my guess is it
was omitted here as the TB pipeline's GFF file didn't include a 'source'
annotation. I don't think 'source' is actually a valid GFF ID and I
suspect we've just been applying the INSDC/GenBank term to GFF files,
but it is one of the two fields parsed by `load_features` and there are
GFF files in Nextstrain build pipelines which use it. Modifying the
underlying `load_features` would be a better solution, but that's a
bigger project for another day.

We additionally update the error message to use the same feature name we
export.

Closes #591
Supersedes #1109
jameshadfield added a commit that referenced this issue Dec 5, 2023
@jameshadfield
[translate] skip parsing source (nuc) as a feature
357345f 
This check is already in place for non-VCF inputs, and my guess is it
was omitted here as the TB pipeline's GFF file didn't include a 'source'
annotation. I don't think 'source' is actually a valid GFF ID and I
suspect we've just been applying the INSDC/GenBank term to GFF files,
but it is one of the two fields parsed by `load_features` and there are
GFF files in Nextstrain build pipelines which use it. Modifying the
underlying `load_features` would be a better solution, but that's a
bigger project for another day.

We additionally update the error message to use the same feature name we
export.

Closes #591
Supersedes #1109
jameshadfield added a commit that referenced this issue Dec 11, 2023
@jameshadfield
[translate] skip parsing source (nuc) as a feature
84343d8 
This check is already in place for non-VCF inputs, and my guess is it
was omitted here as the TB pipeline's GFF file didn't include a 'source'
annotation. I don't think 'source' is actually a valid GFF ID and I
suspect we've just been applying the INSDC/GenBank term to GFF files,
but it is one of the two fields parsed by `load_features` and there are
GFF files in Nextstrain build pipelines which use it. Modifying the
underlying `load_features` would be a better solution, but that's a
bigger project for another day.

We additionally update the error message to use the same feature name we
export.

Closes #591
Supersedes #1109
@jameshadfield
Copy link
Member

Closed by #1348. Please see comment #1348 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working easy problem Requires less work than most issues priority: high To be resolved before other issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix translate andreott/augur
4 participants
@huddlej @andreott @jameshadfield @TheBready