Rebase parser #111
Rebase parser #111
Conversation
![qlty-cloud-legacy[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
|
Code Climate has analyzed commit f95220d and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 95.4% (95% is the threshold). This pull request will bring the total coverage in the repository to 96.9%. View more on Code Climate. |
|
@Koeng101 Things before merge:
|
Guess gotta do a manual check then... I think the code is pretty good.
If it's only once it might be ok. Not like a monthly thing. No other thoughts really, since you need it for testing.
Ok
I don't think this would be a good idea, since that is literally the name of the file when you download it from Rebase - keeping the name the same as what someone would download it as is a good idea IMO. That's also commented in the large comment^ |
|
@Koeng101 what's the status on the smaller rebase file? Any thoughts on deployment since it would actually need to be shipped with each release? |
TimothyStiles
left a comment
TimothyStiles
left a comment
There was a problem hiding this comment.
Code looks solid. Going to check coverage in a second.
My inline comments revolve around naming and exporting. Essentially. How do we pose this to users and devs?
| } | ||
|
|
||
| // ParseRebase parses the Rebase database into a map of enzymes | ||
| func ParseRebase(file []byte) map[string]Enzyme { |
There was a problem hiding this comment.
Should this be exposed to end users? It seems like this would be better as a private function for Poly developers to use unless NEB asks us to export it for them to use.
There was a problem hiding this comment.
I was thinking of having it exposed for end users in case they want to store the enzymes elsewhere, in which they can just pass bytes in to the function. Kinda similar to the idea of GetEnzymeMap, where you actually just directly download the file and pass it into ParseRebase.
| } | ||
|
|
||
| // ReadRebase returns an enzymeMap from a Rebase data dump | ||
| func ReadRebase(path string) (map[string]Enzyme, error) { |
There was a problem hiding this comment.
ReadRebase makes sense to developers but should we use a name like GetEnzymeMap for those who don't know what rebase is?
There was a problem hiding this comment.
Should we have a function GetEnzymeMap that downloads the file directly from NEB and parses? Seems like we'd be hitting their endpoints quite hard in that case.
There was a problem hiding this comment.
Since it would run on every test.
Closes #101
Added a Rebase parser. We still need a way to convert from the enzyme cut sites to the format used by the cloning simulator, but that will come in another PR.