search

brightway-lca / brightway2-data

Tools for the management of inventory databases and impact assessment methods. Part of the Brightway LCA framework.
https://docs.brightway.dev/
BSD 3-Clause "New" or "Revised" License
11 stars 24 forks source link

Make `rapidfuzz` optional on `sys_platform=emscripten` #179

Closed michaelweinold closed 3 months ago

michaelweinold commented 3 months ago

...until:

is ready, this would fix:

michaelweinold commented 3 months ago

...hmmm, that is annoying. I just added the same condition to my testing package - and the build ran fine there.

michaelweinold commented 3 months ago

...must have been the semicolon.

michaelweinold commented 3 months ago

@cmutel, not sure what black magic I need to re-run the checks on this PR. Otherwise ready for review.

cmutel commented 3 months ago

I think the review comes before CI, though I don't really understand that... Don't we also need to shortcircuit the calls which use rapidfuzz, or am I missing other commits or PRs? It seems like this would just lead to an import error.

michaelweinold commented 3 months ago

Since you answer to my question

Is there already a fallback such that whatever-was-used-before-rapidfuzz is used instead, when rapidfuzz is not available?

Was "Yup", I assumed all those short-circuits would be in place. Maybe I should have had a look at the link you provided...

cmutel commented 3 months ago

OK, CI isn't running because the code didn't change, only the build config.

michaelweinold commented 3 months ago

Commit that introduced rapidfuzz and DamerauLevenshtein.distance:

Since the string_distance.py file is still present, we could simply have bw2data fall back to it if rapidfuzz is not installed?

cmutel commented 3 months ago

Since the string_distance.py file is still present, we could simply have bw2data fall back to it if rapidfuzz is not installed?

Sure, that would work; the API might be slightly different but we can control it in the file we included (i.e. make it fit rapidfuzz).

michaelweinold commented 3 months ago

...ok, I think it's all ready - you're already falling back to the string_distance.py file anyway!