Skip to content

[gyb] check isinstance(result, basestring) before string comparison #7324

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 10, 2017
Merged

[gyb] check isinstance(result, basestring) before string comparison #7324

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion utils/gyb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@

from bisect import bisect

try:
basestring
except NameError:
basestring = str


def get_line_starts(s):
"""Return a list containing the start index of each line in s.
Expand Down Expand Up @@ -716,7 +721,8 @@ def execute(self, context):

# If we got a result, the code was an expression, so append
# its value
if result is not None and result != '':
if result is not None \
or (isinstance(result, basestring) and result != ''):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work in Python 3. I think we try to make all the python code compatible with both.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure gyb would require a lot of changes to work with python3, but do you have a preferred solution?

http://stackoverflow.com/questions/11301138/how-to-check-if-variable-is-string-with-python-2-and-3-compatibility

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiousity I just ran gyb.py --verbose-test with both python and python3. Both succeeded. Which is a bit surprising since doctests contain a lot of print x statements. Anyhow, this looks alright.

Copy link
Contributor Author

@toffaletti toffaletti Feb 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I'm surprised too, pleasantly. The utils/gyb file explicitly referencing python2.7 and discussion in #1535 threw me off. I guess I also expected to see unicode_literals imported if the code was trying to be Python 3 ready, and didn't really pay attention to the rest of the code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

from numbers import Number, Integral
result_string = None
if isinstance(result, Number) and not isinstance(result, Integral):
Expand Down