Skip to content

Variant collection tweaks#24

Merged
iskandr merged 5 commits intomasterfrom
variant-collection-tweaks
Feb 25, 2015
Merged

Variant collection tweaks#24
iskandr merged 5 commits intomasterfrom
variant-collection-tweaks

Conversation

@timodonnell
Copy link
Copy Markdown
Contributor

  • Variants are now stored in a set, not a list. The VariantCollection constructor can take any iterable of variants and will convert it to a set.
  • The __str__ method now gives a short summary with just the variant count. The original __str__ method, which gives a line for each variant, has been moved into the variant_summary method.
  • The reference_names and gene_counts methods have been converted to memoized properties.
  • Remove VariantCollection.drop_duplicates since variants are now stored in a set.
  • Make Variant instances comparable. They are ordered by locus.
  • Update tests

 * Variants are now stored in a set, not a list. The VariantCollection constructor can take any iterable of variants and will convert it to a set.
 * The `__str__` method now gives a short summary with just the variant count. The original `__str__` method, which gives a line for each variant, has been moved into the `variant_summary` method.
 * The `reference_names` and `gene_counts` methods have been converted to memoized properties.
 * Remove VariantCollection.drop_duplicates since variants are now stored in a set.
 * Make Variant instances comparable. They are ordered by locus.
 * Update tests
Comment thread test/test_drop_duplicates.py Outdated
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.

I'm not sure about dropping this test entirely, we still probably want to ensure that the expect behavior happens when you initialize a variant collection with duplicates (though now that expected behavior is just the second assert below).

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.

Good call

@iskandr
Copy link
Copy Markdown
Contributor

iskandr commented Feb 25, 2015

Other than (1) adding a test which explicitly expects dropping duplicates by default and (2) moving sorted into VariantCollection.__iter__, LGTM

Also update tests, and add a test that variant collections do not include duplicate variants.
@timodonnell
Copy link
Copy Markdown
Contributor Author

Updated to address comments, but also added a new commit (d1cd616) making a small change to Variant constructor. Please have a look at that @iskandr

iskandr added a commit that referenced this pull request Feb 25, 2015
@iskandr iskandr merged commit 391e101 into master Feb 25, 2015
@iskandr iskandr deleted the variant-collection-tweaks branch February 25, 2015 17:33
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