Python binding forks and different fixes about cld3 HOT 7 CLOSED

Using the work of everyone here (thank you everyone!) I've tried to combine the change sets into one clean set of commits and put a shiny new wrapper on things, which also sits on PyPI as pycld3.

https://github.com/bsolomon1124/pycld3

Reviews appreciated. Again, I've made my best effort to make sure the incremental changes across different forks are picked up and put together.

from cld3.

@ipla I've fixed these memory leaks in my fork of CLD3. Basically, the elizafox version creates a new model object on each call to get_language and on top of it doesn't clean it up. My fork has both the original functions (but cleans up the objects) and a class called LanguageIdentifier which permits reuse of the model for faster performance.

The fork is iamthebot/cld3

from cld3.

@iamthebot

I believe there's still a small error in your fork.

You use the comparison:

str(res.language) != ident.kUnknown:

This is not doing what you think it is.

Originally, res.language is a CPP string, while ident.kUnknown is a const char array (with value "und").

However, str(res.language) does not do the correct coercion in the same way that str(b"hello") does not decode the string; it just makes a str representation of that bytes object.

>>> str(b"hello")
"b'hello'"
>>> str(b"hello") == "hello"  # No!
False

What is needed here is:

if <bytes> res.language != <bytes> ident.kUnknown:

You can prove this for yourself by throwing this into get_language():

cdef string tst = b"und" 
print(tst)
print(str(tst) == ident.kUnknown)
print(tst.decode("utf-8") == ident.kUnknown)

Then

python3 setup.py build_ext --inplace --quiet && python3 -c 'import cld3; cld3.get_language("hello there!")'

Will produce False, False.

from cld3.

Hi @jasonriesa and @akihiroota87: do the maintainers of google/cld3 have any interest in incorporating Python bindings within this repo, by reviewing and combining the various forks mentioned above?

As a tangentially related change, as a part of those forks, the Chromium dependency was removed. If that wasn't the case, the logical solution might be a git submodule, but since the C source itself has changed in the forks, that becomes difficult.

from cld3.

Thanks @bsolomon1124! I actually just copied that part from the elizafox cld3 fork so I guess many of us had been using this in its broken form for a while lol. The new wrapper looks great and we'll switch to using it soon.

from cld3.

I have been testing the Elizafox/cld3 Python binding and I had severe memory issues. The more sentences I detect, the more memory is used. I don't know if this is an issue in cld3 or in the Python binding specifically.

And given that I cannot open any issue in any of the Python binding forks, I though to report it here.

from cld3.

gcld3 - a Python binding for CLD3 from Google

PyPI: https://pypi.org/project/gcld3/

GitHub: https://github.com/google/cld3/tree/master/gcld3

from cld3.