You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@carreter is asking for a full rewrite there, but I think I disagree. Useful link to the spec.
Feature.GetSequence() always returns a nil error value link This should be fairly easy to fix, it is only referenced once.
Gff.AddFeature() code is misleading and mutates Feature state link This doesn't seem like that big of an issue. We can just do a deep copy of the feature and it should be fine.
Common Genbank Feature.Type values should be enumerated link This should be pretty easy, just adding strings enums in a few places.
These are all nice improvements, but are all are actually kinda simple to implement. The first will take just a couple lines of changes with zero impact on functionality, the second just takes a copy, and the third is just adding some enums.
I do think a refactor could be in place: In particular, it might be easy to split the parseChecks into functions. I think there is MASSIVE room for improvement in the test suite as well - but honestly, the genbank parser works pretty darn good right now, so I am hesitant to spend the time on the 4th refactor when I could be using my time on better things. Will implement fixes to those 3 things though.
The text was updated successfully, but these errors were encountered:
Referencing bebop/poly#434
@carreter is asking for a full rewrite there, but I think I disagree. Useful link to the spec.
These are all nice improvements, but are all are actually kinda simple to implement. The first will take just a couple lines of changes with zero impact on functionality, the second just takes a copy, and the third is just adding some enums.
I do think a refactor could be in place: In particular, it might be easy to split the parseChecks into functions. I think there is MASSIVE room for improvement in the test suite as well - but honestly, the genbank parser works pretty darn good right now, so I am hesitant to spend the time on the 4th refactor when I could be using my time on better things. Will implement fixes to those 3 things though.
The text was updated successfully, but these errors were encountered: