sympy / sympy-bot-old

SymPy pull request helper
http://reviews.sympy.org/
Other
24 stars 16 forks source link

Ignore subprocess' output decoding errors #152

Open mattpap opened 11 years ago

mattpap commented 11 years ago

My xelatex generates output with non UTF8 characters. In effect, symy-bot failed just after testing documentation.

asmeurer commented 11 years ago

Are you sure this is really an error that should be silenced? It sounds like you have a bug with your setup. Does the PDF render fine?

mattpap commented 11 years ago

I think so. I'm surprised that there weren't problems before (maybe latex version isn't tested very often). Generated PDF looks good (special characters and diacritics are all in place). Only output contains "improperly" encoded characters in warning messages.

asmeurer commented 11 years ago

An issue with your terminal then? Both @jrioux (I believe) and myself have been building the PDF just fine.

jrioux commented 11 years ago

Encoding errors are nasty because they bring the whole bot to an halt. I have been running the bot from this branch for a bit and it works for me, so if it works for you too, then let's put it in. But can we please just put the 'ignore' directly in the call to decode() instead of defining a global variable?

asmeurer commented 11 years ago

Is there an option other than ignore that will not kill the bot but will still print the error? My main concern here is masking some future error.

mattpap commented 11 years ago

See http://docs.python.org/2/library/stdtypes.html#str.decode. We can ignore errors, replace invalid byte sequences with 0xfffd (unicode replacement character) or (what I didn't know before) register a callback to handle errors. However, I think the whole idea with decoding process output using terminal encoding is wrong, because we can't assume that a process (like xetex) will respect that (apparently skipping explicit encoding makes decode() work without ignore flag).

asmeurer commented 11 years ago

Oh so I guess this is the same as #139. So I have encountered this before :)

jrioux commented 11 years ago

So how about writing 'ignore' directly in the call to decode() instead of defining the global _decode_error_action and then let's merge this.