-
Notifications
You must be signed in to change notification settings - Fork 14
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
Make biopython restriction open-ended #28
Conversation
This matches what requirements.txt already has. Unpinning the dependency makes it easier to combine `bio` with other packages that require `biopython`
Unfortunately, I must pin the version because biopython introduced a functionality-breaking change in 1.80. There is a workaround as described here: but I have not yet gotten to implement and test that change in |
Do you have a test case that fails with biopython==1.80? If so, I may be able to find time to contribute a patch. |
This is the line that fails: https://github.com/ialbert/bio/blob/master/biorun/align.py#L210 It expects the aligned sequences and a trace. I was told by @Paradoxdruid that the following underscore function would work for 1.80:
but I have not yet tested it. It might work for 1.79 as well. It is an underscore method, and that indicates that its function may change in the future. |
Just chiming in; although I dislike breaking changes, the new behavior actually considerably simplified my code: First, I'm determining biopython version via from Bio import __version__ as bio_version
bio_minor: int = int(bio_version.split(".")[1]) Then, my codepaths: https://github.com/Paradoxdruid/pyllelic/blob/master/pyllelic/quma.py#L390-L412 That much shorter 1.80 implementation still passes all the unit tests and passes manual spot checking. All that to say; it might be worth writing to require |
I'm quite unfamiliar with this package and testing changes. I've added some commits to this PR. Is this what you had in mind? |
Ah splendid fix. Looks like the code had some remnants of a previous implementation that were not even needed. Thanks for tracking that down. The code will be merged shortly and updated on PyPI Looks like all tests pass. You can do
at any time to test the package, though occasionally the remote data changes and then of course those tests can will fail (in those cases the tool shows both a diff to the results and the command one would need to be run to recreate the local test data that failed) |
This matches what requirements.txt already has. Unpinning the dependency makes it easier to combine
bio
with other packages that requirebiopython