Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot install if LC_ALL=C #40

Closed
nikicc opened this issue Nov 14, 2015 · 16 comments
Closed

Cannot install if LC_ALL=C #40

nikicc opened this issue Nov 14, 2015 · 16 comments
Assignees

Comments

@nikicc
Copy link
Contributor

nikicc commented Nov 14, 2015

When the system environment variable LC_ALL=C I cannot install smart_open. The problem is in the dependency httpretty, since setup.py requires the version httpretty==0.8.6 which is know not to work with LC_ALL=C. The error I get is this:

UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 133: ordinal not in range(128)

httpretty fixed this error in version 0.8.8, so I am wondering if it would be possible to to relax the requirement to httpretty>=0.8.6?

I actually discovered this when trying to install gensim, which also did not work since it requires smart_open.

@piskvorky
Copy link
Owner

Oh, wow, not good. Nobody's reported this yet.

A fix will be definitely welcome.

We'll just need to make sure everything works (tests pass) -- I don't even know where httpretty is used (probably indirectly through one of smart_open's dependencies)

nikicc added a commit to nikicc/smart_open that referenced this issue Nov 16, 2015
nikicc added a commit to nikicc/smart_open that referenced this issue Nov 16, 2015
piskvorky added a commit that referenced this issue Nov 16, 2015
Upgraded httpretty version; fixing #40
@tmylk tmylk closed this as completed Dec 16, 2015
@tmylk
Copy link
Contributor

tmylk commented Dec 16, 2015

Fixed by #41

@nikicc
Copy link
Contributor Author

nikicc commented Dec 18, 2015

Can this be reopened, since this does not work in version 1.3.1?

The problem is that in #41 (that should fix this) the version was set incorrectly to httpretty>=0.8.8. It should actually be set to httpretty>=0.8.12, since httpretty version 0.8.12 is the first version that can be installed with LC_ALL=C.

@piskvorky
Copy link
Owner

Yes, if the fix doesn't work, please reopen.

@nikicc
Copy link
Contributor Author

nikicc commented Dec 18, 2015

I would, but cannot. Can you?

@piskvorky piskvorky reopened this Dec 18, 2015
@piskvorky
Copy link
Owner

Sorry :)

@piskvorky
Copy link
Owner

@tmylk : some progress in gabrielfalcao/HTTPretty#278 .

@nikicc
Copy link
Contributor Author

nikicc commented Mar 25, 2016

After a quick check httpretty 0.8.14 seems to work ok with python3 and LC_ALL=C (at least locally). Is there any other reason why we still require httpretty to be fixed? Could we relax the condition to allow httpretty 0.8.14?

@tmylk
Copy link
Contributor

tmylk commented Mar 28, 2016

Thanks for the update. I tried httpretty 0.8.14 on with Python 3.4 and it failed same way as in #59 . Raised gabrielfalcao/HTTPretty#291

@nikicc
Copy link
Contributor Author

nikicc commented Mar 28, 2016

OK, I see. I just test the installation part (which seems to be fixed), but forgot to run the tests. Thanks for the effort!

@tmylk
Copy link
Contributor

tmylk commented Mar 29, 2016

Update: Both httpretty==0.8.12 and httpretty==0.8.14 give test errors in smart_open

httpretty==0.8.14 issue reported here
gabrielfalcao/HTTPretty#291

@nikicc
Copy link
Contributor Author

nikicc commented Mar 29, 2016

Thanks for the update! I hope this is fixed in the next version of httpretty, so we can update and also fix #40.

@nikicc
Copy link
Contributor Author

nikicc commented May 13, 2016

I believe this is fixed since httpretty was removed from our dependencies by merging #69.

Could we also release a new version to pip?

@tmylk
Copy link
Contributor

tmylk commented May 13, 2016

Before it is released, can you test if the github version works with LC_ALL=C?

@nikicc
Copy link
Contributor Author

nikicc commented May 13, 2016

I already did and it seem to work OK. If you want to be sure, and re-test it yourself, I did the following:

  1. python setup.py sdist to build the package.
  2. virtualenv t123 to create empty virtual env and activate it with . t123/bin/activate.
  3. LC_ALL=C pip install dist/smart_open-1.3.3.tar.gz

@nikicc
Copy link
Contributor Author

nikicc commented May 20, 2016

This has been fixed with #69 and it works in the new release version 1.3.3 😄

@nikicc nikicc closed this as completed May 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants