Different behaviour with open

[menshikh-iv]

Problem: smart_open==1.6.0 doesn't work correctly with numpy.frombuffer function (numpy < 1.13.0, preferable to test with numpy==1.11.3, python2.7).

Example:
env:

• numpy==1.11.3
• gensim==3.4.0
• smart_open==1.6.0

from gensim.models import FastText
from gensim.test.utils import datapath

m = FastText.load_fasttext_format(datapath("lee_fasttext"))

Trace

IOError                                   Traceback (most recent call last)
<ipython-input-1-afdc56d6c4b5> in <module>()
      2 from gensim.test.utils import datapath
      3 
----> 4 m = FastText.load_fasttext_format(datapath("lee_fasttext"))

/home/ivan/release/gensim/gensim/models/fasttext.py in load_fasttext_format(cls, model_file, encoding)
    697             model_file += '.bin'
    698         model.file_name = model_file
--> 699         model.load_binary_data(encoding=encoding)
    700         return model
    701 

/home/ivan/release/gensim/gensim/models/fasttext.py in load_binary_data(self, encoding)
    712             self._load_model_params(f)
    713             self._load_dict(f, encoding=encoding)
--> 714             self._load_vectors(f)
    715 
    716     def _load_model_params(self, file_handle):

/home/ivan/release/gensim/gensim/models/fasttext.py in _load_vectors(self, file_handle)
    819 
    820         self.num_original_vectors = num_vectors
--> 821         self.wv.vectors_ngrams = np.fromfile(file_handle, dtype=dtype, count=num_vectors * dim)
    822         self.wv.vectors_ngrams = self.wv.vectors_ngrams.reshape((num_vectors, dim))
    823         assert self.wv.vectors_ngrams.shape == (

IOError: first argument must be an open file

If I replace

with utils.smart_open(self.file_name, 'rb') as f:

from https://github.com/RaRe-Technologies/gensim/blob/875b028ccd009fe2fa7665e177b8ab0b5e2dc40d/gensim/models/fasttext.py#L711
to

with open(self.file_name, 'rb') as f:

this will works correctly (also, it works correctly with smart_open==1.5.6, i.e. bug from 1.6.0).

[piskvorky]

Let me add that the bug is the violation of the smart_open contract:

"For local uncompressed files, smart_open() is always a drop-in replacement for open(). There is never a reason not to use smart_open() instead of open()."

[mpenkov]

We have two important contracts: 1. Quack like open (as you mentioned above) 2. Support both Py2 and Py3 Unfortunately, these two contracts conflict because the open functions differ between the major Py versions. Py3 accepts many more parameters and support for encodings. How should we manage this conflict?

…

On Fri, Jul 6, 2018 at 1:07 Ivan Menshikh ***@***.***> wrote: Assigned #207 <#207> to @mpenkov <https://github.com/mpenkov>. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#207 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABDOVHT75SqPjFlhm5uj7cyLG8pwJJkIks5uDjmkgaJpZM4VEJr0> .

[menshikh-iv]

@mpenkov "quack" like a python3 always maybe (this still "contradiction", but looks like best solution)?

[mpenkov]

That’s going to be hard. Look at this example: open a local file with koi8-r encoding. Under Py3, you just call the built-in open and return a file descriptor. This satisfies the contract that Radim mentioned. Everyone is happy. Under Py2, the built-in open does not support encodings, so you have to deal with it yourself. You _could_ wrap it in a codec layer, but then you’d violate the contract: the return would not be a true file descriptor. What should we do in this particular case?

…

On Fri, Jul 6, 2018 at 10:29 Ivan Menshikh ***@***.***> wrote: @mpenkov <https://github.com/mpenkov> "quack" like a python3 always (this still "contradiction", but looks like best solution)? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#207 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABDOVEg3lvKeqxRH6cePs4IkAwbaMRiCks5uDr1_gaJpZM4VEJr0> .

[piskvorky]

smart_open should work like open for local uncompressed files (no matter which Python).

If Python 2 doesn't support some parameters, we don't have to support them either. We have no contract or ambition to provide some "compatibility layer" between python 2 and 3. We want minimum surprises.

[menshikh-iv]

@piskvorky open from python2 or python3? I agree with @mpenkov, this isn't clear: for example, we'll simply use standard python open - we'll have no encoding argument for python2, that's not our goal.

Probably, this should be affected only for trivial variants, i.e. if user pass parameters matched to
https://docs.python.org/2/library/functions.html#open (on python2) or
https://docs.python.org/3/library/functions.html#open (on python3) - we should return open(...), otherwise (non-trivial case) - use current "mechanism"

[piskvorky]

Not sure I understand. What I'm saying is that if native open doesn't support a parameter, neither do we.

I mean, if we support something extra, that's nice. But definitely not at the cost of breaking the "drop-in contract".

[mpenkov]

We _do_ support an encoding parameter, regardless of what the built-in open supports. That’s part of the problem.

…

On Sat, Jul 7, 2018 at 01:16 Radim Řehůřek ***@***.***> wrote: Not sure I understand. What I'm saying is that if native open doesn't support a parameter, neither do we. I mean, if we support something extra, that's nice. But definitely not at the cost of breaking the "drop-in contact". — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#207 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABDOVDX8hp-0Kq6nQnjWlOPLJ3BU4-0Kks5uD416gaJpZM4VEJr0> .

[mpenkov]

Going back to the example I gave previously, if we receive an encoding parameter when the native open doesn’t support one (Py2), we can: 1) perform our own decoding, violating the contract 2) ignore the parameter, but keep to the contract 3) explode with an error 4) ... anything else? How do you think we should handle the above situation?

…

On Sat, Jul 7, 2018 at 08:45 Misha Penkov ***@***.***> wrote: We _do_ support an encoding parameter, regardless of what the built-in open supports. That’s part of the problem. On Sat, Jul 7, 2018 at 01:16 Radim Řehůřek ***@***.***> wrote: > Not sure I understand. What I'm saying is that if native open doesn't > support a parameter, neither do we. > > I mean, if we support something extra, that's nice. But definitely not at > the cost of breaking the "drop-in contact". > > — > You are receiving this because you were mentioned. > > > Reply to this email directly, view it on GitHub > <#207 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABDOVDX8hp-0Kq6nQnjWlOPLJ3BU4-0Kks5uD416gaJpZM4VEJr0> > . >

[piskvorky]

Both 1) and 3) are OK. 1) doesn't really violate the "drop-in" contract, since there is no original to violate (open wouldn't even work with the unknown parameter).

Note however that this is not the case that triggered this issue. There was no encoding parameter -- the original open worked fine.

[mpenkov]

(so27) ubuntu@LAPTOP-S7M90K9N:~/git/smart_open$ pip install numpy==1.11.3
Collecting numpy==1.11.3
  Downloading https://files.pythonhosted.org/packages/68/e8/d68e1c47a2bd3f03cb81f1e635a55b776e51e4746f7204d541f6e3e9ffab/numpy-1.11.3-cp27-cp27mu-manylinux1_x86_64.whl (15.4MB)
    100% |████████████████████████████████| 15.4MB 705kB/s
Installing collected packages: numpy
  Found existing installation: numpy 1.14.5
    Uninstalling numpy-1.14.5:
      Successfully uninstalled numpy-1.14.5
Successfully installed numpy-1.11.3
(so27) ubuntu@LAPTOP-S7M90K9N:~/git/smart_open$ python integration-tests/test_208.py
Traceback (most recent call last):
  File "integration-tests/test_208.py", line 25, in <module>
    loaded = np.fromfile(fin)
IOError: first argument must be an open file
(so27) ubuntu@LAPTOP-S7M90K9N:~/git/smart_open$ pip install numpy==1.14.5
Collecting numpy==1.14.5
  Using cached https://files.pythonhosted.org/packages/6a/a9/c01a2d5f7b045f508c8cefef3b079fe8c413d05498ca0ae877cffa230564/numpy-1.14.5-cp27-cp27mu-manylinux1_x86_64.whl
Installing collected packages: numpy
  Found existing installation: numpy 1.11.3
    Uninstalling numpy-1.11.3:
      Successfully uninstalled numpy-1.11.3
Successfully installed numpy-1.14.5
(so27) ubuntu@LAPTOP-S7M90K9N:~/git/smart_open$ python integration-tests/test_208.py
OK
(so27) ubuntu@LAPTOP-S7M90K9N:~/git/smart_open$

I can reproduce this problem with numpy-1.11.3. With newer numpy, it's no longer a problem.

[menshikh-iv]

Fixed by #208