Initial support for Python 3.5#2
Conversation
|
1.0.2 doesn't pass pypy on my machine for the same reasons. IDK why the build passed a year ago. The python3.5 tests pass, but aren't run by your Travis setup. |
|
Hey nocko! Sorry for my delayed response! Life has been hectic and I've been traveling for work for the past week. Thanks so much for your contribution! I have not been following Twisted for a while, so I was really pleasantly surprised to see how far along the port to Python 3 was. I'll take a look at this soon and get back to you. Thanks again! |
escattone
left a comment
escattone
left a comment
There was a problem hiding this comment.
I have two suggested changes so far.
|
|
||
| def add(self, text): | ||
| lines = (self.remains + text).splitlines() | ||
| lines = (self.remains + text.decode('latin-1')).splitlines() |
There was a problem hiding this comment.
text will be in bytes and is coming from the worker process' stderr (which is a different i/o channel than the one sending pickled results), and is most likely encoded as "utf-8" I believe (unless the worker process explicitly encodes its stderr as something else), so my thought is that text.decode() (using the default encoding of "utf-8") is better here.
| out_safe.write('%d:%s,' % (len(pickled), pickled)) | ||
| out_safe.write((u'{}:{},'.format( | ||
| len(pickled), | ||
| pickled.decode('latin-1'))).encode('latin-1')) |
There was a problem hiding this comment.
out_safe is already opened in binary mode, and I believe pickled is of type bytes, so I'd like to stay with bytes formatting (bytes only supports the % operator). This avoids decoding pickled into unicode for the formatting operation, and then encoding it again into bytes for the write. Also, I'd like to explicitly indicate bytes with the b prefix. I'm using %s instead of %b due to this code having to support Python 2 as well as Python 3.
out_safe.write(b'%d:%s,' % (len(pickled), pickled))2 participants
No description provided.