search

OpenTreeOfLife / opentree

Opentree browsing and curation web site. For overarching or cross-repo concerns, please see the 'germinator' repo.
http://tree.opentreeoflife.org/
BSD 2-Clause "Simplified" License
111 stars 26 forks source link

Bulk TRNS hangs on names with no matches #1232

Open snacktavish opened 4 years ago

snacktavish commented 4 years ago

I tried running these names https://github.com/McTavishLab/OT2020/blob/master/unmatched_mammal_names.txt through the bulk TNRS, and it hangs when it gets to "Pipistrellus alaschanicus" it hangs.

Which we don't have a match for according to:

curl -X POST https://api.opentreeoflife.org/v3/tnrs/match_names -H "content-type:application/json" -d '{"names":["Pipistrellus alaschanicus"],"do_approximate_matching":true}'

bredelings commented 4 years ago

Thanks! I will take a look.

bredelings commented 4 years ago

Oh, thoughts on whether this is a backend issue, or an opentree issue?

snacktavish commented 4 years ago

Pretty confident it’s an opentree issue. Behaves fine on direct api calls.

jimallman commented 4 years ago

Found it! There's a PR on opentree that watches for new behavior in bulk TNRS.

Its code is in otcetera, right? I'm guessing the problem is in how this json object is populated. If there's no match of any kind, it look like this defaults to null instead of an empty array.

jimallman commented 4 years ago

It occured to me that returning null here might be a best practice for JSON, in which case my JS changes were the only remedy needed. A quick search suggests that there's no consensus here, but many voices lean toward returning an empty array.

bredelings commented 4 years ago

I'll just fix this to return an empty array.

bredelings commented 4 years ago

I fixed this on the optimize-dp branch for now: https://github.com/OpenTreeOfLife/otcetera/commit/684d62cde8da2f148deb44dcb45fa9a3ef80b71c

jimallman commented 4 years ago

Does the optimize-dp branch have a special status vs. develop? Or can I merge it there, for a standard push to devapi?

bredelings commented 4 years ago

It looks like api is pulling from the faster-search branch, so that we can merge to development without affecting production. I think we'd eventually like to get back to the point where development becomes master after we successfully deploy it to production.

So, yes, I think that's fine.

bredelings commented 4 years ago

I just merged that into development.

snacktavish commented 4 years ago

Ah, I think related to this if you search terms that don't match in the taxon browser you get no response. e.g. https://tree.opentreeoflife.org/taxonomy/browse?name=dinodaur

whereas on dev, where this fix is implemented, you get that nothing is matched e.g. https://tree.opentreeoflife.org/taxonomy/browse?name=dinodaur

Should be fixed when development branch of otcerta is merged to master and deployed.