search

dgasmith / opt_einsum

⚡️Optimizing einsum functions in NumPy, Tensorflow, Dask, and more with contraction order optimization.
https://dgasmith.github.io/opt_einsum/
MIT License
863 stars 68 forks source link

modify python_requires in setup.py to require python 3.6+ #126

Closed cdepillabout closed 4 years ago

cdepillabout commented 4 years ago

The setup.py file sets python_requires='>=3.5':

https://github.com/dgasmith/opt_einsum/blob/4323cfce5f415dc425143f216171cefa56d5d429/setup.py#L25

However, opt_einsum is using random.choices, which is only available on >=python-3.6:

https://github.com/dgasmith/opt_einsum/blob/4323cfce5f415dc425143f216171cefa56d5d429/opt_einsum/path_random.py#L263

random.choices api docs: https://docs.python.org/3/library/random.html#random.choices

Also, as far as I can tell, travis is attempting to test with python-3.5:

https://github.com/dgasmith/opt_einsum/blob/4323cfce5f415dc425143f216171cefa56d5d429/.travis.yml#L12

https://github.com/dgasmith/opt_einsum/blob/4323cfce5f415dc425143f216171cefa56d5d429/.travis.yml#L21-L24

But these tests are actually running with python-3.6:

dgasmith commented 4 years ago

Thanks for the report, this is indeed a mistake and a failure with the current CI setup. For fixing this particular issue I believe we can replace with with np.random.choice. I think this is 1-1, @jcmgray can you comment?

We are due for a release so we should be able to patch this up and get a release out in the next few days.

jcmgray commented 4 years ago

Yep I can fix this shortly - the reason to use random.choices is speed, so if there are no objections I'll try and import that version first then resort to numpy.