Fix bug with pytaxonkit.name #26
Conversation
| wget ftp://ftp.ncbi.nih.gov/pub/taxonomy/taxdump.tar.gz | ||
| curl -L -O ftp://ftp.ncbi.nih.gov/pub/taxonomy/taxdump.tar.gz |
There was a problem hiding this comment.
Looks like GitHub Actions Mac VM no longer has wget installed by default.
| idlist = '\n'.join(map(str, ids)) | ||
| if idlist == '': | ||
| idlist = '\n'.join(map(str, ids)) + '\n' | ||
| if idlist == '\n': |
There was a problem hiding this comment.
The power of the newline
| def test_name_regression(): | ||
| result = name([6]) | ||
| print(result) | ||
| assert len(result) == 1 | ||
| assert result.Name[0] == 'Azorhizobium' |
There was a problem hiding this comment.
Added a regression test.
|
Hi @philippbayer, can you confirm this fix works for you? |
|
I can confirm it now works on my system!! Thanks so much |
|
Investigated the Popen bufsize parameter as a potential alternative solution, but wasn't able to get it to work. I'm happy with this fix in any case. I'm pretty sure the changes in this PR already address the cause of the monthly cron CI failure. Ready for review and merge! |
There was a problem hiding this comment.
Can confirm that the changes fix the bug described in #26. Also confirmed that the tests pass.
|
LGTM! Thank you! |
Small bug fix for
pytaxonkit.name. Also, looks like some Symplocos-ologist has been busy at work since pytaxonkit was last updated, so made some minor changes to the expected output of the test suite.Closes #25.