biocore / deblur

Deblur is a greedy deconvolution algorithm based on known read error profiles.
BSD 3-Clause "New" or "Revised" License
92 stars 41 forks source link

bugfix: 0 read fasta raised an error instead of continue #76

Closed amnona closed 8 years ago

amnona commented 8 years ago

following move to scikit-bio, when encountering a 0 read file (after msa), deblur raised an error instead of continuing with a warning. fixed to write a warning and continue (so samples with 1 read will not stop the deblurring)

mortonjt commented 8 years ago

Looks like there is a failing test

amnona commented 8 years ago

oops. forgot to fix the unittest. should work now (test warn instead of raise)

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.08%) to 89.412% when pulling dc9ce06e68ba12bfafd44012c949bc0fe771d915 on amnona:develop into 675f7fbbbd5a79b319ae8264d4c5fb6986fb676c on biocore:master.

amnona commented 8 years ago

ok, now passes all checks :)

amnona commented 8 years ago

Also, can we push this change to the pip install (and bump up the version :) )

mortonjt commented 8 years ago

I'd hold off on the pip install - it gets confusing if there are too many releases. Looks like the change is rather small - so I'd suggest waiting until a whole bunch of patches are made to shoot off another release.

And the dev version is still pip installable via

pip install git+https://github.com/biocore/deblur.git

So it won't be a problem pip installing the dev version. This can be added to the readme if necessary.

mortonjt commented 8 years ago

good to merge once comments are addressed

mortonjt commented 8 years ago

Forgot to mention.

Now we are post alpha release, it is advantageous to start squashing commits. It'll simplify the process of debugging using tools such as git bisect.

You can do this by

git rebase -i 675f7fb
# rebase all commits except the first commit with `squash`
git push -f

Note that this is optional, but it will clean up the commit history.

wasade commented 8 years ago

Can a change log mention be added please?

amnona commented 8 years ago

Fixed (except specific warn msg text testing in unit_test) tried to squash (not sure it worked)

amnona commented 8 years ago

Since this is a BUGFIX, i think we should immediately update the pip (since the version is already out). Don't see any advantage in waiting?

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.1%) to 89.437% when pulling 53922d50998ee7cc8c3a597ed8de98c20b573e33 on amnona:develop into 675f7fbbbd5a79b319ae8264d4c5fb6986fb676c on biocore:master.

wasade commented 8 years ago

@amnona, do you perceive this as impacting most users or just a small percent? If the latter, then we might want to hold off another few days to see if any other bugs pop up which we should fix immediately.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.1%) to 89.437% when pulling 4585d7bac3c9a6f9ee0939858cd270ed831f3631 on amnona:develop into 675f7fbbbd5a79b319ae8264d4c5fb6986fb676c on biocore:master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.1%) to 89.437% when pulling 102f99bd637fdf324317bf4b0085651478473731 on amnona:develop into 675f7fbbbd5a79b319ae8264d4c5fb6986fb676c on biocore:master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.1%) to 89.437% when pulling 102f99bd637fdf324317bf4b0085651478473731 on amnona:develop into 675f7fbbbd5a79b319ae8264d4c5fb6986fb676c on biocore:master.

amnona commented 8 years ago

ready for merge :)

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.1%) to 89.437% when pulling f20fee3c7f98cfd45c3403833cec40417bfb4d66 on amnona:develop into 675f7fbbbd5a79b319ae8264d4c5fb6986fb676c on biocore:master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.1%) to 89.437% when pulling bcfeb2b27c4719d7f3c4dc778641d34f5db698a9 on amnona:develop into 675f7fbbbd5a79b319ae8264d4c5fb6986fb676c on biocore:master.