classification
Title: Doctests in test_dictcomp depend on dict order
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.4, Python 3.3, Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Frank.Wierzbicki, eric.araujo, ezio.melotti, fwierzbicki, python-dev, r.david.murray, serhiy.storchaka, terry.reedy
Priority: normal Keywords: easy, patch

Created on 2013-01-07 17:12 by fwierzbicki, last changed 2013-01-16 21:59 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
test_dictcomp.patch fwierzbicki, 2013-01-07 17:12 review
dictcomp3.3.patch fwierzbicki, 2013-01-10 20:12 review
dictcomp2.7.patch fwierzbicki, 2013-01-10 20:15
dictcomps_updated.patch fwierzbicki, 2013-01-12 02:34 review
dictcomps_updated2.patch fwierzbicki, 2013-01-12 19:22 Address more comments review
Messages (20)
msg179272 - (view) Author: Frank Wierzbicki (fwierzbicki) * (Python committer) Date: 2013-01-07 17:12
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.
msg179350 - (view) Author: √Čric Araujo (eric.araujo) * (Python committer) Date: 2013-01-08 13:37
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.
msg179368 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-08 20:03
It would be good actually get rid of doctests, turning them into unittests.
msg179501 - (view) Author: Frank Wierzbicki (fwierzbicki) * (Python committer) Date: 2013-01-09 23:49
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.
msg179591 - (view) Author: Frank Wierzbicki (fwierzbicki) * (Python committer) Date: 2013-01-10 20:12
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 :)
msg179592 - (view) Author: Frank Wierzbicki (fwierzbicki) * (Python committer) Date: 2013-01-10 20:15
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"
msg179668 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-01-11 11:09
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.
msg179723 - (view) Author: Frank Wierzbicki (fwierzbicki) * (Python committer) Date: 2013-01-11 19:57
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?
msg179724 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-01-11 20:12
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...)
msg179740 - (view) Author: Frank Wierzbicki (fwierzbicki) * (Python committer) Date: 2013-01-11 22:46
> 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.
msg179750 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-01-12 01:15
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.
msg179752 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-01-12 01:49
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.
msg179754 - (view) Author: Frank Wierzbicki (fwierzbicki) * (Python committer) Date: 2013-01-12 02:34
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?
msg179756 - (view) Author: Frank Wierzbicki (Frank.Wierzbicki) Date: 2013-01-12 02:45
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 :)
msg179767 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-01-12 05:02
> 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).
msg179775 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-01-12 07:50
The hg workbench gui of tortoisehg, which I use, has make/apply patch selections that get translated to hg commands. So same thing ;-).
msg180059 - (view) Author: Frank Wierzbicki (fwierzbicki) * (Python committer) Date: 2013-01-15 22:03
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?
msg180060 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-01-15 22:13
If you addressed last Ezio's comments, it will be good to me.
msg180062 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-01-15 22:25
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).
msg180112 - (view) Author: Roundup Robot (python-dev) Date: 2013-01-16 21:59
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
History
Date User Action Args
2013-01-16 21:59:06python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg180112

resolution: fixed
stage: patch review -> resolved
2013-01-15 22:25:31r.david.murraysetmessages: + msg180062
2013-01-15 22:13:09serhiy.storchakasetmessages: + msg180060
2013-01-15 22:03:34fwierzbickisetmessages: + msg180059
2013-01-12 19:22:36fwierzbickisetfiles: + dictcomps_updated2.patch
2013-01-12 07:50:26terry.reedysetmessages: + msg179775
2013-01-12 05:02:36ezio.melottisetmessages: + msg179767
2013-01-12 02:45:05Frank.Wierzbickisetnosy: + Frank.Wierzbicki
messages: + msg179756
2013-01-12 02:34:09fwierzbickisetfiles: + dictcomps_updated.patch

messages: + msg179754
2013-01-12 01:49:35r.david.murraysetmessages: + msg179752
2013-01-12 01:15:52terry.reedysetnosy: + terry.reedy
messages: + msg179750
2013-01-11 22:46:07fwierzbickisetmessages: + msg179740
2013-01-11 20:12:01r.david.murraysetnosy: + r.david.murray
messages: + msg179724
2013-01-11 19:57:16fwierzbickisetmessages: + msg179723
2013-01-11 11:09:06ezio.melottisetversions: + Python 2.7, Python 3.2
nosy: + ezio.melotti

messages: + msg179668

type: behavior
2013-01-10 20:15:06fwierzbickisetfiles: + dictcomp2.7.patch

messages: + msg179592
2013-01-10 20:13:04fwierzbickisetfiles: + dictcomp3.3.patch

messages: + msg179591
2013-01-09 23:49:12fwierzbickisetmessages: + msg179501
2013-01-08 20:03:02serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg179368
2013-01-08 13:37:07eric.araujosetversions: + Python 3.3
nosy: + eric.araujo

messages: + msg179350

keywords: + easy
stage: patch review
2013-01-07 17:12:14fwierzbickicreate