search

Martinsos / edlib

Lightweight, super fast C/C++ (& Python) library for sequence alignment using edit (Levenshtein) distance.
http://martinsos.github.io/edlib
MIT License
492 stars 162 forks source link

Issue84 #192

Closed GDCCP closed 2 years ago

GDCCP commented 2 years ago

I think the indents could be off as I am using a temp VM and haven't configured my emacs properly (I think it is not replacing TABs).

I haven't done a github pull request before, so feel free to tell me what I haven't done right.

I am using std::unique_ptr and std::vector internally and only use malloc when returning 'char *' to avoid breaking script language bindings.

Martinsos commented 2 years ago

Related to https://github.com/Martinsos/edlib/issues/84 .

Thanks for the PR @GDCCP !!

This is a pretty big PR and I haven't been working with CPP actively for the last couple of years so I am pretty rusty with it -> I will need to take a proper amount time to go through it and check it out, so I apologize for the wait upfront. I will also probably be asking a decent amount of questions here to flush out the reasoning for specific changes.

One thing that might help is if you could break this PR into multiple smaller PRs, each one of them taking care of a separate thing. I am not sure if it makes sense to break it that way, if it can be nicely done, so if not then never mind and I will try to review it as it is. EDIT: Oh I see now it is broken into multiple commits. That already helps!

GDCCP commented 2 years ago

I will try to break up the work into multiple PRs, and open an issue about the memory leak.

I could be tied up in the next few weeks('til end of Nov) though. Don't be surprised if you don't hear from me for a few days.