#28012 closed task (fixed)
py3: import certain things from collections.abc
Reported by:  slelievre  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  python3  Keywords:  days101, py3, collections.abc 
Cc:  chapoton, jhpalmieri, slelievre, vdelecroix, tscrim  Merged in:  
Authors:  Samuel Lelièvre  Reviewers:  John Palmieri 
Report Upstream:  N/A  Work issues:  
Branch:  4f80136 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
Background (read Python 3 documentation for the `collections` module for more detail):
 Collections Abstract Base Classes
("CABCs") moved to the
collections.abc
module in Python 3.3.
 These "Collections Abstract Base Classes" are:
AsyncGenerator
,AsyncIterable
,AsyncIterator
,Awaitable
,Bytestring
,Callable
,Collection
,Container
,Coroutine
,Generator
,Hashable
,ItemsView
,Iterable
,Iterator
,KeysView
,Mapping
,MappingView
,MutableMapping
,MutableSequence
,MutableSet
,Reversible
,Sequence
,Set
,Sized
,ValuesView
.
 For backwards compatibility, CABCs continue to be visible
in the
collections
module through Python 3.8; and can be imported either fromcollections
or fromcollections.abc
.
 Starting with Python 3.9, CABCs must be imported from
collections.abc
.
 In Python 2 they could only be imported from 'collections',
not from 'collections.abc'. So while Sage was keeping
Py2 and Py3 compatible it made sense to import from
collections
.
 In Python 3.7 and Python 3.8, importing them from
collections
gives a deprecation warning. This warning was silenced in Sage, see #28002.
In this ticket we import the classes from collections.abc
.
We do not suppress the silencing of the deprecation warning yet. See comment:23 and #30768.
Change History (36)
comment:1 Changed 23 months ago by
 Branch set to u/slelievre/collections.abc
comment:2 Changed 23 months ago by
 Commit set to e04fa482bab365d19cbc5946800728b3fc382345
 Keywords py3 added
comment:3 Changed 23 months ago by
 Cc chapoton jhpalmieri added
 Status changed from new to needs_review
comment:4 Changed 23 months ago by
Note: to check where such changes were needed I followed advice in a comment at #28002 and did
$ git grep "from collections import"
comment:5 Changed 23 months ago by
 Status changed from needs_review to needs_work
More changes are needed, as revealed by
$ git grep "collections\."
I'll push a commit.
comment:6 Changed 23 months ago by
The relevant lines in the output of git grep "collections\."
seem to be:
src/sage/arith/misc.py: if isinstance(ks[0], collections.Iterable): src/sage/combinat/finite_state_machine.py: collections.Iterator): src/sage/combinat/ranker.py: from collections.abc import Iterable, Sequence src/sage/combinat/ranker.py: tuple (or more generally a :class:`collections.Sequence`), an src/sage/combinat/ranker.py: :class:`collections.Iterable`). See :trac:`15919`. src/sage/combinat/ranker.py: Lists, tuples, and other :class:`sequences <collections.Sequence>`:: src/sage/combinat/shuffle.py: assert(isinstance(l1, collections.Iterable) and src/sage/combinat/shuffle.py: isinstance(l2, collections.Iterable) src/sage/combinat/shuffle.py: assert(all(isinstance(elem, collections.Iterable) for elem in l1)) src/sage/combinat/shuffle.py: assert(all(isinstance(elem, collections.Iterable) for elem in l2)) src/sage/combinat/shuffle.py: assert(isinstance(l1, collections.Iterable) and src/sage/combinat/shuffle.py: isinstance(l2, collections.Iterable) src/sage/databases/conway.py:class DictInMapping(collections.Mapping): src/sage/databases/conway.py:class ConwayPolynomials(collections.Mapping): src/sage/game_theory/normal_form_game.py: from collections.abc import MutableMapping src/sage/geometry/cone.py: collections.Hashable, src/sage/geometry/cone.py: collections.Iterable): src/sage/geometry/cone.py: collections.Container): src/sage/geometry/fan.py: collections.Callable, src/sage/geometry/fan.py: collections.Container): src/sage/geometry/lattice_polytope.py:class LatticePolytopeClass(SageObject, collections.Hashable): src/sage/geometry/lattice_polytope.py: collections.Hashable): src/sage/matrix/matrix_integer_sparse.pyx: from collections.abc import Iterator, Sequence src/sage/matrix/matrix_modn_sparse.pyx: from collections.abc import Iterator, Sequence src/sage/misc/converting_dict.py: if isinstance(arg, collections.Mapping): src/sage/modules/with_basis/morphism.py: if not isinstance(diagonal, collections.Callable): src/sage/plot/colors.py:class Colormaps(collections.MutableMapping): src/sage/plot/point.py: if isinstance(points, collections.Iterator): src/sage/repl/ipython_kernel/interact.py: from collections.abc import Iterable, Iterator src/sage/repl/ipython_kernel/widgets_sagenb.py: from collections.abc import Iterable, Sequence
comment:7 Changed 23 months ago by
Two comments: there may be instances of this in other packages, like matplotlib. Also, it's not just warnings silenced in #28002, but an older silencing in sage/all.py
.
comment:8 Changed 23 months ago by
 Description modified (diff)
comment:9 Changed 23 months ago by
 Description modified (diff)
comment:10 Changed 22 months ago by
 Milestone changed from sage8.8 to sage8.9
Moving tickets from the Sage 8.8 milestone that have been actively worked on in the last six months to the next release milestone (optimistically).
comment:11 Changed 16 months ago by
 Milestone changed from sage8.9 to sage9.1
Ticket retargeted after milestone closed
comment:12 Changed 12 months ago by
 Milestone changed from sage9.1 to sage9.2
Moving tickets to milestone sage9.2 based on a review of last modification date, branch status, and severity.
comment:13 Changed 9 months ago by
 Milestone changed from sage9.2 to sage9.3
comment:14 Changed 9 months ago by
Now that we don't support py2 any more, the try/except approach is not needed any more.
comment:15 Changed 9 months ago by
 Branch changed from u/slelievre/collections.abc to public/ticket/28012collections.abc
 Commit changed from e04fa482bab365d19cbc5946800728b3fc382345 to 2187c31c9dac5a4d53cfcf5a8e271ed46d03fac3
 Keywords collections.abc added
 Status changed from needs_work to needs_review
Sorry for letting over a year pass by. Here is a new branch. Please review.
The file finite_state_machine.py
has import collections
and then collections.defaultdict
, collections.OrderedDict
,
collections.deque
, collections.namedtuple
, collections.abc.Iterator
.
Would there be an advantage, or a disadvantage, to doing the following instead:
from collections import defaultdict, deque, namedtuple, OrderedDict from collections.abc import Iterator
and then using defaultdict
, OrderedDict
, deque
, namedtuple
, Iterator
?
I changed to that style in some other files.
New commits:
2187c31  t28012: Collections Abstract Base Classes

comment:16 Changed 9 months ago by
 Commit changed from 2187c31c9dac5a4d53cfcf5a8e271ed46d03fac3 to 29faddf94c784f0a08e0ad7a106c9f3e3a2c990b
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
29faddf  t28012: Collections Abstract Base Classes

comment:17 Changed 9 months ago by
I had left one file out, I forced push.
comment:18 Changed 9 months ago by
 Description modified (diff)
comment:19 Changed 9 months ago by
 Cc vdelecroix added
Vincent, I now see you already did the work at #28315 a year ago.
Sorry for delaying this. Would you like to review?
comment:20 Changed 8 months ago by
 Cc tscrim added
comment:21 Changed 8 months ago by
 Commit changed from 29faddf94c784f0a08e0ad7a106c9f3e3a2c990b to 489eccff3e15a229539b6086f5b5276dee366888
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
489eccf  t28012: Collections Abstract Base Classes

comment:22 Changed 8 months ago by
comment:23 Changed 8 months ago by
I'm getting some deprecation warnings. Some can be fixed by this:

src/sage/repl/ipython_kernel/interact.py
diff git a/src/sage/repl/ipython_kernel/interact.py b/src/sage/repl/ipython_kernel/interact.py index 5fb318a6e4..ac0376ca63 100644
a b EXAMPLES:: 35 35 36 36 from ipywidgets.widgets import SelectionSlider, ValueWidget, ToggleButtons 37 37 from ipywidgets.widgets.interaction import interactive, signature 38 from collections import Iterable, Iterator, OrderedDict 38 from collections.abc import Iterable, Iterator 39 from collections import OrderedDict 39 40 from .widgets import EvalText, SageColorPicker 40 41 from .widgets_sagenb import input_grid 41 42 from sage.structure.element import parent
Some are coming from other Python packages, though. (This is with a Sage build using a system Python 3.7.) For example, one doctest fails because of
File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage9.2.beta11/local/lib/python3.7/sitepackages/babel/localedata.py", line 17, in <module> from collections import MutableMapping
another with
File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/TESTING/sage9.2.beta11/local/lib/python3.7/sitepackages/requests/packages/urllib3/util/selectors.py", line 11, in <module> from collections import namedtuple, Mapping
Maybe we need to delay the suppression of the silencing of the deprecation warning. ;)
comment:24 Changed 8 months ago by
 Status changed from needs_review to needs_work
I agree:
src/repl_ipython_kernel/interact.py
does need the change you say suppressing the silencing of the warnings does need to be delayed
I will take care of that. Any advice about my question in 15?
comment:25 Changed 8 months ago by
The typical (but not universal) Sage style seems to be "from blah import foo" and then "foo(...)" rather than "import blah" and then "blah.foo(...)". So that's what I would choose, as you suggested.
comment:26 Changed 8 months ago by
 Commit changed from 489eccff3e15a229539b6086f5b5276dee366888 to 1ee0124092f2759a2958905fb19402bca9bcffec
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
1ee0124  t28012: Collections Abstract Base Classes

comment:27 Changed 8 months ago by
 Commit changed from 1ee0124092f2759a2958905fb19402bca9bcffec to 0ce9e4043a959f5f2a21ad59f7179a2485c47575
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
0ce9e40  t28012: Collections Abstract Base Classes

comment:28 Changed 8 months ago by
 Status changed from needs_work to needs_review
Fixed and rebased on 9.2.beta12. Sorry for one more forcepush.
comment:29 Changed 8 months ago by
 Status changed from needs_review to needs_work
Still needs work. Almost there. Will push shortly.
comment:30 Changed 8 months ago by
 Commit changed from 0ce9e4043a959f5f2a21ad59f7179a2485c47575 to 4f8013611b5c27658e3ba49aea0bf1b4066b4adb
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4f80136  t28012: collections.abc

comment:32 Changed 8 months ago by
 Reviewers set to John Palmieri
 Status changed from needs_review to positive_review
Looks good to me.
comment:33 Changed 8 months ago by
 Branch changed from public/ticket/28012collections.abc to 4f8013611b5c27658e3ba49aea0bf1b4066b4adb
 Resolution set to fixed
 Status changed from positive_review to closed
comment:34 Changed 8 months ago by
 Milestone changed from sage9.3 to sage9.2
comment:35 Changed 7 months ago by
 Commit 4f8013611b5c27658e3ba49aea0bf1b4066b4adb deleted
Followup at #30768 for unsilencing the deprecation warning.
comment:36 Changed 7 months ago by
 Description modified (diff)
New commits:
#28012 import from collections.abc