Tweaks to _AltRecord for possible multiple inheritance #64

Merged
merged 3 commits into from Jul 2, 2012

Conversation

Projects
None yet
3 participants

lennax commented Jul 1, 2012

A few minor changes to the inheritance pattern of _AltRecord and children. super() needs to be called even for objects that inherit from object to allow multiple inheritance, and every class passing **kwargs allows mixing of subclasses with custom args.

Also made _AltRecord into an abstract base class to enforce overloading of __str__.

An example of multiple inheritance I can imagine in this context would be, for example, a class UnknownLoc that could be combined with a variety of Alt types. Rough example: https://gist.github.com/3029028

References about use of super:
Praise: http://rhettinger.wordpress.com/2011/05/26/super-considered-super/
Caution: https://fuhm.net/super-harmful/

lennax commented Jul 2, 2012

I'll tack on a third commit:

The super() call in Reader still refers to the legacy VCFReader. It appears to work, but I changed it just to be sure. The pre-3 requirement of explicit declaration of the class name and self as args to super() is frustrating...

Also changed method parseALT to _parse_alt for pep8/consistent style.

@jamescasbon jamescasbon pushed a commit that referenced this pull request Jul 2, 2012

James Casbon Merge pull request #64 from lennax/lenna
Tweaks to _AltRecord for possible multiple inheritance
6e91990

@jamescasbon jamescasbon merged commit 6e91990 into jamescasbon:master Jul 2, 2012

Maybe a constructor can be removed, since it doesn't have anything else besides a super call?

The constructor passes an arg to the superclass. Also, the rest of the classes in the project use super(), and my goal here was to allow for possible multiple inheritance. I provided some examples and links in pull request 64.

Why CamelCase for keyword arguments, when most of the code uses pythonic under_score?

@gotgenes gotgenes pushed a commit to gotgenes/PyVCF that referenced this pull request May 13, 2014

James Casbon Merge pull request #64 from lennax/lenna
Tweaks to _AltRecord for possible multiple inheritance
494917f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment