search

sckott / pytaxize

python port of taxize (taxonomy toolbelt) for R
https://sckott.github.io/pytaxize/
MIT License
34 stars 13 forks source link

Delete temporary name list file in _gnr_resolve() #27

Closed lyttonhao closed 9 years ago

lyttonhao commented 9 years ago

The temporary file should be removed.

sckott commented 9 years ago

@lyttonhao thanks. Please add a test in tests directory to this PR testing that the file is removed

lyttonhao commented 9 years ago

Already added.

panks commented 9 years ago

@lyttonhao I believe in your test you are trying to invoke GNR resolver with POST where it sends a file. You don't need to keep the size of file to 1010 names, as anyway gnr_resolve() will slice the list into smaller sizes (<=1000) and then pass it to _gnr_resolve(). Anything with size > 300 would go via POST protocol using a file. So you can reduce the size of file containing name (to 301 names), otherwise it would take unnecessary time during test.

panks commented 9 years ago

@sckott I have replaced xrange with range in gnr, in my current PR. If you restart the build on CI after that, this PR will pass for python3 as well.

lyttonhao commented 9 years ago

@panks Thanks for your advice. I have changed the query number to 301 in test_gnr_resolve_remove_temporary_file(), and added a new test for testing when queried number is larger than 1000. So I think the large species names file is still needed.

sckott commented 9 years ago

I've restarted the build...

sckott commented 9 years ago

thanks @lyttonhao , but your test failed on python 3.4 https://travis-ci.org/sckott/pytaxize/jobs/56682501

Did you test on python 3?

lyttonhao commented 9 years ago

@sckott , I'm sorry I don't test on python 3.4. I will have a look at it soon.