Benjamin Peterson added the comment:
People pointed out in #21295 that this made some things that were possible
before impossible, so the lineno and col_offset changes of this have been
reverted.
--
___
Python tracker rep...@bugs.python.org
Roundup Robot added the comment:
New changeset 7d1c32ddc432 by Benjamin Peterson in branch '3.4':
revert lineno and col_offset changes from #16795 (closes #21295)
https://hg.python.org/cpython/rev/7d1c32ddc432
--
___
Python tracker
Benjamin Peterson added the comment:
Hi Sven, I was about to apply this (sorry for the delay), and I realized
there's one more thing. We have an example AST unparser in Tools/parser that
needs to be updated for AST changes. You can run it's test suite by running
test_tools in the main test
Sven Brauch added the comment:
Hi Benjamin,
the delay is not a problem -- as long as this is in time for Python 3.4,
everything is fine.
Attached is a patch which adjusts the unparser to the changes. Acoording to the
tests, this is all that needs to be updated.
Cheers,
Sven
--
Roundup Robot added the comment:
New changeset 7c5c678e4164 by Benjamin Peterson in branch 'default':
unify some ast.argument's attrs; change Attribute column offset (closes #16795)
http://hg.python.org/cpython/rev/7c5c678e4164
--
nosy: +python-dev
resolution: - fixed
stage: -
Roundup Robot added the comment:
New changeset 219c997b880b by Benjamin Peterson in branch 'default':
add Sven Brauch for his #16795 contribution
http://hg.python.org/cpython/rev/219c997b880b
--
___
Python tracker rep...@bugs.python.org
Sven Brauch added the comment:
Thanks for reviewing this, and thanks for guiding me through the implementation
process. ;)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
Sven Brauch added the comment:
I don't want to push anything, but did you find time to review this yet? It
would be great to have it in the next release.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
Brett Cannon added the comment:
Python 3.4.0a1 isn't due until August so you have no worries about missing the
next release. =)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
Sven Brauch added the comment:
Any news on this yet? ;)
Unfortunately, I'm still having no luck in adding the patch to the review tool
(same error message).
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
Benjamin Peterson added the comment:
Are you attaching files directly on Rietveld or on this issue?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
Sven Brauch added the comment:
Attaching files to this bug report here works fine (see corrected patch above),
but when I add the file to http://bugs.python.org/review/16795/ under Add
another patchset, I get the error message I described. I tried with firefox,
chromium and konqueror.
Benjamin Peterson added the comment:
Yeah, I think that's broken. It's best just to attach them here.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
Ezio Melotti added the comment:
It's just not supported -- the Add another patchset link should be removed
from rietveld.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
Sven Brauch added the comment:
Oh, alright, that explains things. In this case, the file I attached on Jan 29
(http://bugs.python.org/file28905/81300-change-var-kwargs-new.diff) should
contain all the requested changes.
Greetings
--
___
Python
Changes by Benjamin Peterson benja...@python.org:
--
assignee: - benjamin.peterson
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
___
Sven Brauch added the comment:
Hm, I'm still getting the same error messages from the review tool which I
described earlier; I can neither comment nor add patches.
So, I'll have to abuse the bug report again:
Thanks for the review. Is it possible you selected the wrong patch file for the
Éric Araujo added the comment:
I'm not sure how to make hg include a commit message in the patch...
See hg help export.
(In Mercurial, the only objects are changesets; hg log trawls through commit
messages (with options to see short text, full text, diff), hg diff only shows
diff, and hg
Ezio Melotti added the comment:
It's not necessary to include the commit message and/or use hg export though,
since we don't import patches directly and we write the message ourself when we
commit.
(... is there a better way to upload three files?)
You need to upload them individually and
Sven Brauch added the comment:
I have signed the contributor agreement and sent a scan to the specified mail
address (received no reply so far, but I guess that's okay).
Did anyone happen to find the time to look at the patches yet?
Greetings,
Sven
--
Benjamin Peterson added the comment:
Thanks for signing the agreement. I'll try to look at the patches by the end of
this weekend. Sorry for the delay.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
Sven Brauch added the comment:
Here's the next version which I hope to be somewhat complete now.
vararg and kwarg are now of type arg, and I did all the changes which are
required to make this possible. The ast tests pass.
Do you prefer to have this as one large patch all together, or would
Benjamin Peterson added the comment:
Individual patches would be great.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
___
Sven Brauch added the comment:
Alright, I'll be back with those shortly (as soon as I found out how to do this
best with hg -- I'm used to git ;). I'll also sign the contributor agreement,
that's no problem of course.
--
___
Python tracker
Sven Brauch added the comment:
Okay, here they are. I'm not sure how to make hg include a commit message in
the patch...
81299-extend-asdl.diff: Changes required to the ASDL framework, in order to
allow attributes ( ... ) on a product
81300-change-var-kwargs.diff: Makes var/kwarg be instances
Sven Brauch added the comment:
second patch file
--
Added file: http://bugs.python.org/file28788/81300-change-var-kwargs.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
Sven Brauch added the comment:
third patch file
(... is there a better way to upload three files?)
--
Added file: http://bugs.python.org/file28789/81301-change-attr-ranges.diff
___
Python tracker rep...@bugs.python.org
Benjamin Peterson added the comment:
I think None should be treated as meaning not present for an optional
argument.
By the way, it would be good if we could get you to sign a contributor
agreement. http://www.python.org/psf/contrib/
--
___
Python
Sven Brauch added the comment:
I think I got it mostly working now (it was quite simple in fact), but there's
one issue which I can't seem to solve. This fails:
compile(ast.parse(def fun(): pass), file, exec)
Traceback (most recent call last):
File stdin, line 1, in module
TypeError:
Sven Brauch added the comment:
Not an issue, having this thing resolved upstream would save a huge lot of pain
elsewhere. ;)
So, to make sure... I'll go to the asdl file, make arguments have two arg
attributes which store the data for the var and kwarg which they can contain,
then I adjust
Benjamin Peterson added the comment:
Yes. Feel free to do that in separate patches as needed.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
Sven Brauch added the comment:
Here's another version now which I think could be used like this. All tests
have been adjusted. I'll append two patches, one just containing the changes to
the parser for ease of review, and one full diff which also contains changes to
the generated files and
Sven Brauch added the comment:
Attached is the full diff this time.
--
Added file: http://bugs.python.org/file28680/full.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
Sven Brauch added the comment:
The patch review tool currently throws errors on submitting any form
(http://pastie.org/pastes/5665048/text) so please forgive me for answering here
once more. I'll copy this information (patch + message) to the review as soon
as the website is working again.
Benjamin Peterson added the comment:
Could you post an example of the error, please?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
Sven Brauch added the comment:
The above post has an example for trying to add a patch, here's what happens
when I try to post a reply: http://pastie.org/pastes/5665144/text
I also tried with another web browser, so it's unlikely that it's the browser's
fault (but maybe the user's? ;)
Benjamin Peterson added the comment:
Ah, sorry, I was talking about the failure of optional arguments.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
Sven Brauch added the comment:
Ah, whops, I misunderstood that.
The error is rather generic:
Traceback (most recent call last):
File ./Lib/test/test_ast.py, line 796, in test_lambda
self._check_arguments(fac, self.expr)
File ./Lib/test/test_ast.py, line 596, in _check_arguments
Benjamin Peterson added the comment:
Ah, yes. This is part of the annoying inconsistency in our asdl framework.
Here's what I think should happen:
- on arguments, vararg and kwarg should get the arg type, killing some of the
numerous fields on arguments
- asdl needs to be hacked, so arg can
Sven Brauch added the comment:
Here's a new proposal, I adjusted the AST tests and fixed some small problems I
encountered during that. It contains all the diffs for generated files, should
I remove those for easier review?
A remaining problem is that AST_Tests::_assertTrueorder now fails, I
Sven Brauch added the comment:
While writing tests, I noticed that the additional fields (lineno, col_offset
for vararg, kwarg, and other arguments) currently are mandatory. Is that a
problem?
It doesn't seem trivial to change that, since apparently only attributes (not
fields) can be
Benjamin Peterson added the comment:
A question mark after the type name in the AST makes it optional.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
Sven Brauch added the comment:
Thanks. I had seen and tried this before, but the ast module in python, which
is used in the tests, still requires the additional arguments. Probably this is
only valid for the C API?
--
___
Python tracker
Benjamin Peterson added the comment:
It would be good if
a) the patch was against hg default
b) it had tests
--
nosy: +benjamin.peterson
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
Changes by Brett Cannon br...@python.org:
--
nosy: +brett.cannon
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
___
Python-bugs-list
Changes by Meador Inge mead...@gmail.com:
--
nosy: +meador.inge
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
___
Python-bugs-list
New submission from Sven Brauch:
Here's a patch doing some adjustments to the AST to make it more useful for
static language analysis, as discussed in
http://mail.python.org/pipermail/python-dev/2012-December/123320.html.
Changes done:
* the described fix to attribute ranges
* add location
Changes by Eric Snow ericsnowcurren...@gmail.com:
--
nosy: +eric.snow
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16795
___
___
Python-bugs-list
48 matches
Mail list logo
