[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[Terry J. Reedy]

Terry J. Reedy  added the comment:

yes ;-)

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[miss-islington]

miss-islington  added the comment:


New changeset ac60d1afd2b04f61fe4c965740fa32809f2b84ed by Miss Islington (bot) 
in branch '3.7':
bpo-32411: IDLE: Remove line number sort in browser.py (GH-5011)
https://github.com/python/cpython/commit/ac60d1afd2b04f61fe4c965740fa32809f2b84ed


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[miss-islington]

Change by miss-islington :


--
pull_requests: +13616
pull_request: https://github.com/python/cpython/pull/13732

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[Terry J. Reedy]

Terry J. Reedy  added the comment:


New changeset 1a4d9ffa1aecd7e750195f2be06d3d16c7a3a88f by Terry Jan Reedy 
(Cheryl Sabella) in branch 'master':
bpo-32411: IDLE: Remove line number sort in browser.py (#5011)
https://github.com/python/cpython/commit/1a4d9ffa1aecd7e750195f2be06d3d16c7a3a88f


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[Raymond Hettinger]

Raymond Hettinger  added the comment:

Can this move forward now?

--
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[Terry J. Reedy]

Terry J. Reedy  added the comment:

I should mention that using Treeview could mean not using pyclbr.Instead, 
we might extract the parsing loop and modify it to build tree nodes as 
functions and classes are encountered.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[Terry J. Reedy]

Terry J. Reedy  added the comment:

The patch is trivial, and I assume that the test change is adequate, but I want 
to put this issue on hold for the present because
a) the performance gain is minuscule (the code cleanup is worth a bit more);
b) I am reluctant to backport something to 3.6 that depends on an unofficial 
implementation change, but not doing so will make backports of changes in this 
area not automatic;
c) I think I want to switch the browsers to ttk.Treeview, I hope before 3.6 
goes off maintenance, and I expect that doing so will change this area of the 
code, and possibly the line being changed.

'The present' ends when 3.6 maintenance ends in 6 months or at least the module 
browser is patched, whichever is first.

--
stage: test needed -> patch review
versions: +Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[Cheryl Sabella]

Cheryl Sabella  added the comment:

Terry,

I'm not sure if I phrased my initial question correctly.  I've made a pull 
request to show you what I was thinking.

What I had tried to show with the test case change is that, prior to the 
guarantee of insertion order on the dictionary, the check `eq(tcl, [C0, f0])` 
would have failed without the sort because `transform_children` would have 
returned `[f0, C0]`.  With the sort in place or with the guarantee insertion 
order, this test doesn't fail.

Thanks!

--
stage: patch review -> test needed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[Cheryl Sabella]

Change by Cheryl Sabella :


--
keywords: +patch
pull_requests: +4900
stage: test needed -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[Terry J. Reedy]

Terry J. Reedy  added the comment:

(f0, C0) is the correct insertion and line number order for the dict that would 
be returned by pyclbr for a fleshed-out file.  After 'sorted' is removed, the 
mock must imitate that.

One could argue that previously, and even now for 3.6, the initial insertion 
order should be 'wrong' to test that format_children sorts the return list.  
But what is the chance that there will ever be an alternate 3.6 (or even later) 
implementation that runs tkinter and therefore might run IDLE but has a dict 
that does not preserve insert order?

I probably should not worry about the dict insert order guarantee being 
rescinded in a future version.  Maybe instead I should just add a comment that 
transform_children depends on iteration order being insert order == line-number 
order.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[Cheryl Sabella]

Cheryl Sabella  added the comment:

Terry,

In test_browser, would it be a good test if mock_pyclbr_tree was created as 
`mock_pyclbr_tree = {'C0': C0, 'f0': f0}` 
instead of 
`mock_pyclbr_tree = {'f0': f0, 'C0': C0}`?

The reason that I'm asking is that, under 2.7 and 3.3, this dictionary does not 
retain the insertion order, but changes it from (C0, f0) to (f0, C0).  

Or did you just want a test that checks that the line numbers are still in 
order when the code is returned from transform_children?

--
nosy: +csabella

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32411] Idlelib.browser: stop sorting dicts created by pyclbr

[Terry J. Reedy]

New submission from Terry J. Reedy :

pyclbr does a linear scan of a file and returns a dict, and Class objects have 
a dict of children. For objects in the file being scanned, insertion order is 
the same as line number order.  idlelib.browser.transform children filters out 
objects not in the file, changes the .name of some objects in the file, appends 
to a list, and sorts by line number.  (I should have done the sort in place.)  
For insertion order dicts, the sort does nothing. In 3.7, insertion order is a 
language guarantee, not just a (current) CPython implementation detail.  The 
sort does not change the order and is therefore no longer needed. We can reduce
return sorted(obs, key=lambda o: o.lineno)
to
return obs

However, I do want to make sure that there is at least one test that will break 
if there is a bug somewhere.

--
assignee: terry.reedy
components: IDLE
messages: 308945
nosy: terry.reedy
priority: normal
severity: normal
stage: test needed
status: open
title: Idlelib.browser: stop sorting dicts created by pyclbr
type: performance
versions: Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com