[2.7] bpo-28143: ASDL compatibility with Python 3 system interpreter by malthe · Pull Request #4082 · python/cpython (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Hello, and thanks for your contribution!
I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).
Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.
Thanks again to your contribution and we look forward to looking at it!
[2.7] ASDL compatibility with Python 3 system interpreter [2.7] bpo-28143: ASDL compatibility with Python 3 system interpreter
| @@ -1194,7 +1194,7 @@ def main(srcfile): |
|---|
| sys.exit(1) |
| if INC_DIR: |
| p = "%s/%s-ast.h" % (INC_DIR, mod.name) |
| f = open(p, "wb") |
| f = open(p, "w") |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "wb" mode is used for disallowing newlines translation. In Python 3 you should use newline='\n'.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is Python 2.7.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then why this is changed?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently this doesn't work neither with Python 2 nor with Python 3.
@serhiy-storchaka sorry, I had some trouble applying the original patch so it was only partially complete. It should work now.
| @@ -1194,7 +1194,7 @@ def main(srcfile): |
|---|
| sys.exit(1) |
| if INC_DIR: |
| p = "%s/%s-ast.h" % (INC_DIR, mod.name) |
| f = open(p, "wb") |
| f = open(p, "w") |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then why this is changed?
| except Exception, err: |
|---|
| print "Error visiting", repr(object) |
| print err |
| except Exception as err: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This syntax is not compatible with Python 2.5. Use sys.exc_info() for more compatibility.
| print string.join(rhs[:pos]), |
|---|
| print '.', |
| print string.join(rhs[pos:]) |
| print('\t\t', lhs, '::=', end=' ') |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
end= is not supported in Python 2.5. Use the single print().
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @@ -644,7 +645,7 @@ def buildTree(self, nt, item, tokens, k): |
|---|
| attr[i] = self.buildTree(sym, why[0], |
| tokens, why[1]) |
| item, k = self.predecessor(key, why) |
| return self.rule2func[self.new2old[rule]](attr) |
| return self.rule2func[self.new2old[rule]](*attr) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the call in deriveEpsilon() be updated too?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so yes. Fixed in e14e536.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.
Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
Python 3 emits a warning:
/home/serhiy/py/cpython2.7/Parser/asdl.py:387: ResourceWarning: unclosed file <_io.TextIOWrapper name='./Parser/Python.asdl' mode='r' encoding='UTF-8'>
buf = open(file).read()
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the code compatible with Python 2.5, fix the problem with a resource warning and newlines translation.
@csabella, I've made the necessary changes. I don't know if the continuous integration test actually runs this code. I assume that there isn't anything set up for running the build with a default Python 3 interpreter since this is what was failing.