-
-
Notifications
You must be signed in to change notification settings - Fork 73
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.
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
Genbank parser needs heavy refactor or a rewrite #434
Comments
Dropping a link to the GenBank spec here: https://www.ncbi.nlm.nih.gov/genbank/release/current/ |
That's a neat looking format 🤔 Happy to take a whack at a rewrite. A few questions:
|
|
re 1. Oh nice! I don't want to step on your toes- so no pressure. Maybe there's room to divy up different parts of the spec after establishing how the header will be parsed? If so, I'd be happy to help with that! Also, if it is inappropriate to divy up the work, I'd be happy to help build out 400 instead. That or any other issue that would help out Poly that is higher priority and or high difficulty. re 2. I think targeting the latest version of the spec is definitely the better option. Adding backwards compat after the fact seems straight forward in this case. It doesn't look like there are too many breaking changes and they seem pretty separate from everything else. |
Not stepping on any toes :) I'll try and get that code up today, I'm away from the PC I had the code on. Feel free to take a stab at any issues you would like!!! |
Ok, draft PR is up that has the types + most of the layout of the parsers logic. @TwFlem let me know what you think! |
@carreter That looks super good! That pattern will go along with way the genbank parser along with the other parsers as well. The error type looks super good as well. It looks flexible enough so we can get nice and specific about what it wrong. The way things are being done just looks good in general. The only real value added suggestions are about testing. The others are more speculation. |
This issue has had no activity in the past 2 months. Marking as |
I hath returned from my slumber... will be trying to work on this over the coming week or two. The parser is pretty much complete and passes unit tests, but I'd like to add write functionality and confirm that the parser works after a round-trip before merge. |
This issue has had no activity in the past 2 months. Marking as |
Describe the desired feature/enhancement
The
io/genbank
package needs a significant refactor or to be re-written entirely.Is your feature request related to a problem?
There are several outstanding issues with the parser that will be difficult to address in its current state: #383 #352 #351 #342
Describe alternatives you've considered (optional)
Main alternative is to hack over the code as-is to fix bugs, but I think this will take more time than a rewrite.
The text was updated successfully, but these errors were encountered: