lebedov / msgpack-numpy

Serialize numpy arrays using msgpack
Other
197 stars 33 forks source link

0.4.4.1 breaks spacy #33

Closed anneschuth closed 5 years ago

anneschuth commented 6 years ago

Since 0.4.4.1 we have breaking builds of projects that use spacy, with the following error:

    ents_model_nl.to_disk(model_path_nl)
../../../virtualenv/python3.6.3/lib/python3.6/site-packages/spacy/language.py:621: in to_disk
    util.to_disk(path, serializers, {p: False for p in disable})
../../../virtualenv/python3.6.3/lib/python3.6/site-packages/spacy/util.py:503: in to_disk
    writer(path / key)
../../../virtualenv/python3.6.3/lib/python3.6/site-packages/spacy/language.py:609: in <lambda>
    ('tokenizer', lambda p: self.tokenizer.to_disk(p, vocab=False)),
tokenizer.pyx:354: in spacy.tokenizer.Tokenizer.to_disk
    ???
tokenizer.pyx:355: in spacy.tokenizer.Tokenizer.to_disk
    ???
tokenizer.pyx:384: in spacy.tokenizer.Tokenizer.to_bytes
    ???
../../../virtualenv/python3.6.3/lib/python3.6/site-packages/spacy/util.py:486: in to_bytes
    return msgpack.dumps(serialized, use_bin_type=True, encoding='utf8')
../../../virtualenv/python3.6.3/lib/python3.6/site-packages/msgpack_numpy.py:196: in packb
    return Packer(**kwargs).pack(o)
E   TypeError: __init__() got an unexpected keyword argument 'encoding'
honnibal commented 6 years ago

(spaCy author here.)

Happy to fix our version pin, or update our code? I think that function was written with reference to a previous version of msgpack (when it was named msgpack-python). It looks like I need to simply remove that encoding argument?

anneschuth commented 6 years ago

Ah, thanks @honnibal, thats great!

At the same time, this may have broken other packages too, I guess this (backwards incompatible) changes should maybe be part of a major release?

lebedov commented 6 years ago

@honnibal - yes, the encoding argument is deprecated in msgpack-python 0.5.2+; removing it should fix the problem.

lebedov commented 5 years ago

Since msgpack-numpy is now bundled in srsly, I assume this is no longer an issue. Feel free to reopen if needed.