-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
Properly handle chained exceptions in Python 3 #216
Conversation
if sys.version_info.major == 2: | ||
from StringIO import StringIO | ||
else: | ||
from io import StringIO |
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.
Why not have this import at the top of the file? Might also want to do except ImportError:
instead so you don't need sys
if you do it at the top.
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.
Why not have this import at the top of the file?
I put it here locally mostly because this is function is rarely used. If you prefer I can move it to the top though, no problem.
Might also want to do except ImportError: instead so you don't need sys if you do it at the top.
In general I tend to avoid that because it might lead to hard to diagnose errors (see pytest-dev/pytest-mock#68).
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.
@The-Compiler gentle ping. 😁
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.
Fair point - seems fine to keep it here.
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.
Thanks!
Fix #215