Skip to content

collect effect annotation errors in dictionary, only look up overlapping...#13

Merged
iskandr merged 2 commits intomasterfrom
catch_variant_annotation_exceptions
Feb 17, 2015
Merged

collect effect annotation errors in dictionary, only look up overlapping...#13
iskandr merged 2 commits intomasterfrom
catch_variant_annotation_exceptions

Conversation

@iskandr
Copy link
Copy Markdown
Contributor

@iskandr iskandr commented Feb 17, 2015

... transcripts and construct the Gene objects later

…ing transcripts and construct the Gene objects later
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should raise_on_error be in the VariantAnnotator constructor instead of peppered across different methods?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most common use case is VariantCollection which uses VariantAnnotator under the hood. Not sure how to economize on raise_on_error since you'll need one arg in VariantCollection and another in VariantAnnotator.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I figured VariantCollection would pass it through to VariantAnnotator. The constructor just felt more appropriate to me, based on knowing far less about the code than you do.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructor feels a little clumsier to me since the typical usecase seems to be:

  1. Construct variant_collection = VariantCollection(some_vcf_path)
  2. Call variant_collection.variant_effects()
  3. Encounter an error
  4. Decide the error is irrelevant
  5. Call variant_collection.variant_effects(raise_on_error=False) to ignore the error

Having to create a new VariantCollection seems like a very minor hassle but it also doesn't have a clear payoff.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@tavinathanson
Copy link
Copy Markdown
Contributor

A few comments, but generally looks good to me!

iskandr added a commit that referenced this pull request Feb 17, 2015
…ions

collect effect annotation errors in dictionary, only look up overlapping...
@iskandr iskandr merged commit 66ca061 into master Feb 17, 2015
@iskandr iskandr deleted the catch_variant_annotation_exceptions branch February 17, 2015 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants