Opened 6 years ago

Closed 5 months ago

#15981 closed enhancement (fixed)

Python 3 preparation: Adapt dict-methods keys(), items(), values() etc.

Reported by: wluebbe Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: python3 Keywords: python3
Cc: embray, jdemeyer, fbissey, tscrim, jhpalmieri, vklein Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by wluebbe)

Python 3 introduces new dict-methods and changes the behavior of existing.

Changes according to lib2to3/fixes/fix_dict.py:

d.keys() -> list(d.keys())
d.items() -> list(d.items())
d.values() -> list(d.values())

d.iterkeys() -> iter(d.keys())
d.iteritems() -> iter(d.items())
d.itervalues() -> iter(d.values())

d.viewkeys() -> d.keys()
d.viewitems() -> d.items()
d.viewvalues() -> d.values()

TODO: Clarify whether some generated changes need modifications to avoid unnecessary wrappings with list().

This ticket is tracked as a dependency of meta-ticket ticket:16052.

Change History (26)

comment:1 Changed 6 years ago by wluebbe

Here are the (approximate) numbers for the dict-methods in the Sage .py modules:

case: .xxxx() case: .iterxxxx()
xxxx (method) nbr lines nbr calls nbr lines nbr calls
keys() 414 430 19 19
values() 216 216 15 15
items() 284 330 293 295

No .viewxxxx() were found

The effect of 2to3 fix_dict is:

  • d.xxxx() is converted to list(d.xxxx()).
    • For Py2.7 the original code gives a list.
      The converted code gives also a list.
      But the converted code performs an EXTRA dictionary copy!
    • For Py3.3 the original code would be a view (not a list) and therefor different from the Py2.7 behavior.
      The converted code gives a list (as in Py 2.7 for both code variants).
  • d.iterxxxx() is converted to iter(d.xxxx()).
    • For Py2.7 the original code gives an iterator.
      The converted code results in an iterator -
      but this iterator is created ON TOP of the list returned by d.xxxx().
    • For Py3.3 the original code is not excepted.
      The converted code gives an iterator on a view (and is similar to the iterators in Py 2.7 for both code variants).

In summary

  • the original code in Py 2.7 is equivalent to
  • the converted code in Py 2.7 is equivalent to
  • the converted code in Py 3.3

In both cases (.xxxx() and .iterxxxx()) the converted code suffers an overhead in Py2.7!

The six library module offers a solution without this overhead: it defines 3 new functions: iterkeys(d), itervalues(d), iteritems(d). In Py2.7 they are implemented by d.iterkeys(), d.itervalues(), d.iteritems() and in Py3.3 by d.keys(), d.values(), d.items().
The catch is that the code must be manually modified (use the new functions in place of the old dictinary methods) - and the module six must be imported.

What to do?

  • The approaches (2to3 and six) could be combined.
  • And in some places/contexts the wrapping list(d.xxxx()) could be removed manually when a list is not required.
  • Another option is sometimes to use key in d together with d[key] as appropriate. This code has the same behavior in Py2.7 and Py3.3. But manual code change is required

There is no undisputed best solution :-(
Suggestions are welcome!

comment:2 Changed 6 years ago by wluebbe

If you have been wondering (like me) why 2to3 maps d.iteritems() to iter(d.items()) and what is means in Py3, then you may benefit from this entry in stackoverflow.

comment:3 Changed 6 years ago by ohanar

I would personally move this to stage 2, since most likely we will want to use some sort of compatibility library for at least some instances (and I don't want to make a choice now of which library we will use).

comment:4 Changed 6 years ago by wluebbe

  • Description modified (diff)

comment:5 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:6 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:7 Changed 4 years ago by jdemeyer

  • Component changed from distribution to python3

comment:8 Changed 3 years ago by chapoton

for some work on keys() and iterkeys(), see tickets #21304, #21296, #21266

comment:9 Changed 3 years ago by chapoton

For some work on itervalues, see #21310 and #21320

Last edited 3 years ago by chapoton (previous) (diff)

comment:10 Changed 3 years ago by chapoton

see #22298 for some work on iteritems

comment:11 Changed 3 years ago by chapoton

after #22485, there should remain no .view* and no .iter*.

comment:12 Changed 3 years ago by chapoton

  • Milestone changed from sage-6.4 to sage-8.0

comment:13 Changed 2 years ago by chapoton

  • Milestone changed from sage-8.0 to sage-8.1

Now we face the issue that .keys()[0], .items()[0] and so on are not allowed.

First ticket for .items in #23824

comment:14 Changed 2 years ago by chapoton

next step in #23831

comment:15 Changed 2 years ago by chapoton

next in #24068 for .values()[something]

comment:16 Changed 2 years ago by chapoton

next in #24126 for some remaining .keys()[something]

comment:17 follow-up: Changed 2 years ago by chapoton

  • Cc jdemeyer added

Sigh.. it seems that we will also need to get rid of iteritems and the like in pyx files (python3-built-sage traceback):

sage: v=vector([1,0,2])
sage: list(v.iteritems())
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-29-e4acb1d548e2> in <module>()
----> 1 list(v.iteritems())

/home/chapoton/sage3/src/sage/modules/free_module_element.pyx in sage.modules.free_module_element.FreeModuleElement.iteritems (build/cythonized/sage/modules/free_module_element.c:12200)()

AttributeError: 'dict' object has no attribute 'iteritems'

comment:18 in reply to: ↑ 17 Changed 2 years ago by jdemeyer

Follow-up: #24134

comment:19 Changed 2 years ago by chapoton

next step in #24181

comment:20 Changed 2 years ago by chapoton

next step in #24224

comment:21 Changed 2 years ago by chapoton

next in #24407

comment:22 Changed 19 months ago by chapoton

next in #25259

comment:23 Changed 6 months ago by chapoton

  • Cc embray fbissey tscrim jhpalmieri vklein added
  • Milestone changed from sage-8.1 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

I propose to close this one also. Agreed ?

comment:24 Changed 5 months ago by vklein

Yes i agree. I don't think this meta-ticket is needed anymore.

comment:25 Changed 5 months ago by jhpalmieri

  • Status changed from needs_review to positive_review

comment:26 Changed 5 months ago by chapoton

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.