test_dictcomp hard codes the dict output of various tests of Dict Comprehensions. Since Jython has a different dict ordering we are currently altering this test. When we get to 3.x it will be nicer if we can use this test as is. Also I've seen some discussion on Python-Dev that CPython devs want to get rid of dict ordering in the tests anyway. Patch attached, which assigns the dict values to a variable and compares it in the tests instead of just dumping the output in the doctests.

Yes, we definitely want to eat our dogfood and stop relying on dict order, especially now that hashing is randomized.

I would argue for backporting this to stable branches.

Patch looks good.  A semi-related improvement for this bug would be to switch these doctests to proper unit tests.

It would be good actually get rid of doctests, turning them into unittests.

Making these into real unit tests and switching the patch to 3.3 should be no problem. Is 2.7 still open for changes to tests? I might back port to 2.7 as well so that I can delete the customized Jython test in 2.7.

Switched to unittest style away from doctests and created patch against 3.3. Note that at this time test_dictcomp.py is identical in 3.3 and 3.4 so merging should be pretty easy :)

This patch of test_dict_comp.py was made against 2.7. It differs from the 3.3 version only one line "from test import test_support as support"

I left a review on Rietveld.
Frank, once the comments have been addressed and the patch approved, do you want to commit it on all the 4 branches yourself?
If you prefer I can take care of it.

I'd love to work through the commit process myself once I get through all of the comments. My intention is to become much more active here as Jython3 starts to ramp up over the next year. It would be great if I could eventually get all of Jython's .py files into CPython's Lib (as has been discussed at previous PyCons). Getting all of the tests here would be a solid step in that direction. My plan is to go through this review process for the near future but do the commits myself.

Just to be sure the 4 branches are 2.7, 3.3, 3.4 and default? And can I merge safely from 2.7 to 3.3?

Well, we generally prefer to go through the review process always, except for relatively small commits (which some of yours may be).

2.7 is separate from 3.x.  So to commit to all four branches you commit to 3.2, merge to 3.3, merge to default, and the graft to 2.7.  (Myself, I'm still using patch to do the 2.7 updates, I haven't tried graft yet.)  Before you push, you should have 2.7 and default as active branches (hg branches) but no others.  And if push tells you you are trying to create remote heads, that generally means someone else pushed since your last pull, and you will need to merge their heads with yours before you can push.

(In theory this should all be in the devguide...)

> Well, we generally prefer to go through the review process always, except for relatively small commits (which some of yours may be).

Good to know, in that case I'll plan to go through the review process always.

> 2.7 is separate from 3.x.  So to commit to all four branches you commit to 3.2, merge to 3.3, merge to default, and the graft to 2.7.  (Myself, I'm still using patch to do the 2.7 updates, I haven't tried graft yet.)

When you say "use patch" to do the 2.7 update - do you mean a manual patch? I'm not ready to look into graft and would prefer to just do the regular thing for now.

> (In theory this should all be in the devguide...)
I'll definitely be reading the devguide as I prepare to push my changes.

Yes, export patch from 3.2 or 3.3 (which you need to do anyway to upload here), import patch into 2.7. Feel free to ask questions on the Core-mentorship list. The devguide is still a work-in-progress, but has had several recent patches. Great to hear that Jython3 is on the horizon.

I'm not positive what Terry means by export patch and import patch...I think hg actually has commands like that, but I don't use them myself.  I just use hg diff to generate the diff/patch file, and then use the regular patch command to apply it to 2.7.  (I'm sure there are those who will laugh at me for being old school and not taking advantage of our nice shiny dvcs :)

I hear that graft is better at resolving conflicts, but as I say I haven't tried it yet.

I'm getting an error when I try to upload patches via Reitveld:

TypeError at /review/16886/add
int() argument must be a string or a number, not 'AddForm'

so I'm attaching my updated patch here. Is this a known issue? If not, where should I report it?

On Fri, Jan 11, 2013 at 6:34 PM, Frank Wierzbicki
<report@bugs.python.org> wrote:
> Reitveld
*Rietveld

I see just uploading a new patch to bugs.python does the right thing
so I'll just move along :)

> The devguide is still a work-in-progress

I would say it's already quite mature, however I'm rewriting the part about commiting: see #14468 and https://bitbucket.org/ezio_melotti/devguide/src/default/committing.rst?at=default (in particular the "working with mercurial" part and the "multiple clones approach").

> I'm getting an error when I try to upload patches via Reitveld:

Just upload them here :)

If you have other questions you can also come on IRC (#python-dev on Freenode).

The hg workbench gui of tortoisehg, which I use, has make/apply patch selections that get translated to hg commands. So same thing ;-).

I have addressed all of the comments, but I don't know the exact procedure here. Does someone need to say "Looks good to me" before I push?

If you addressed last Ezio's comments, it will be good to me.

The ideal is if someone says "looks good to me".  The practical path is that if you have addressed all the comments, and no one objects after a couple-three days, you should feel free to commit (unless you feel like you want another review, in which case you can ask for one).

New changeset 6d06b223c664 by Frank Wierzbicki in branch '2.7':
Closed #16886: test_dictcomps no longer depends on dict order
http://hg.python.org/cpython/rev/6d06b223c664

New changeset c7dfc307b88e by Frank Wierzbicki in branch '3.2':
Closed #16886: test_dictcomps no longer depends on dict order
http://hg.python.org/cpython/rev/c7dfc307b88e

New changeset 8ab5bd5f95b3 by Frank Wierzbicki in branch '3.3':
#16886: merge with 3.2
http://hg.python.org/cpython/rev/8ab5bd5f95b3

New changeset d0cef6c90efc by Frank Wierzbicki in branch 'default':
#16886: merge with 3.3
http://hg.python.org/cpython/rev/d0cef6c90efc