-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix issue 326 unicode decode error #374
Fix issue 326 unicode decode error #374
Conversation
Here are the beginnings of a test -- I haven't had a chance to make this a proper test yet:
|
I've made good progress on writing a test for this issue, but then I ran into an issue with ScriptTest: https://bitbucket.org/ianb/scripttest/issue/10/ I have a fix and a pull request for the above - without it, the test will fail. |
Re: the With Python 2, without the ScriptTest fix at
With Python 2, with the ScriptTest fix at
With Python 3, without fix in 412daad:
With Python 3, with fix in 412daad:
|
Anyone get a chance to try this? Just curious if it works for other folks. |
Anyone get a chance to try this? |
@@ -70,8 +70,12 @@ def log(self, level, msg, *args, **kw): | |||
if self.explicit_levels: | |||
## FIXME: should this be a name, not a level number? | |||
rendered = '%02i %s' % (level, rendered) | |||
if hasattr(consumer, 'write'): | |||
consumer.write(rendered+'\n') | |||
if hasattr(consumer, 'buffer'): # Python 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we encapsulate this check into a utility method in pip.backwardcompat? I'd like to avoid sprinkling the codebase with more ad-hoc Python 3 checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, take a look at commit efe4495.
Thanks for this work, it looks really good! Made a couple comments, I'll give it some more thorough testing soon. Sorry I didn't get to it sooner. I've also asked Ian if we can get ScriptTest released, so I can merge this without making pip's tests dependent on an unreleased version of ScriptTest. https://bitbucket.org/ianb/scripttest/issue/11/release |
…ty method (`fwrite`) in pip.backwardcompat. Refs pypa#374 (comment)
Commit efe4495 is my attempt to address carljm's comment about encapsulating a Python 2 vs. Python 3 check. Let me know if that works or if it needs tweaking.
|
Update looks good. I think we're waiting on a ScriptTest release here. |
ScriptTest 1.2 is out, putting this on my list for review and merge. |
So with this branch (merged up to develop), tests pass on Python 3.2, but the new test fails on Python 2.7 with a UnicodeEncodeError. |
Doh. OK, I'll try to take a look at the Python 2.7 test failure soon. |
Hmmm. It seems to work for me. The develop branch didn't have
Then I ran the test:
Here's the version of Python that I was using:
and here's what I happen to have installed:
|
Mmm, I miscommunicated. I haven't merged your branch into develop yet. When I said "merged up to develop" what I meant was that (locally) I merged develop into your branch, so it would be fully up to date with changes in develop in the meantime. So of course develop branch doesn't have the new test file, it doesn't have any of the changes from your branch :-) What your results indicate is that your added test apparently passes with the current code under Python 2.7, but fails with the code changes in your branch; the reverse of the situation with Python 3. We need to update the code so that your test passes under both Python 2 and 3. |
Ah ok. So I need to try rebasing my branch on to develop and then test that. Thanks for the clarification. |
Hmmm. Maybe I'm still not doing what you did.
|
Ok, how embarrassing. I updated my Python 3.2 virtualenv to ScriptTest 1.2 but forgot to do the same for my Python 2.7 env, so it was still running ScriptTest 1.1.1. Everything passes on all versions, 2.4 - 3.2. Merged - thanks very much for the contribution, and sorry for wasting your time with my stupidity! |
Ha ha. No worries! I'm glad that this turned out to be useful. Hope to meet you someday. Maybe at PyCon 2012? |
I'll be there! |
hey @msabramo, the test for this is failing on win py3.3 and here it is with more output any quick thoughts? otherwise, I look at this closer later.
|
From a quick look, I notice that the Windows system is using CP-1252 as its encoding. The test in question emits UTF-8 so that's probably why there is a |
i have this error occur when i run pip-freeze.txt error: command 'x86_64-linux-gnu-gcc' failed with exit status 1 Cleaning up... |
@srinu-dipl please report your problem in a new issue, it's also helpful to say the command you entered and the version of software that you're using (python, pip, virtualenv, operating system) |
This is a proposed fix for #326
412daad - Modify
console_to_str
to handle non-ASCII (UTF-8) input.b154c82 - Modify
Logger.log
so that it can output UTF-8 encoded messages.