Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpo-25988: Emit a warning when use or import ABCs from 'collections'. #5460

Merged
merged 1 commit into from
Jan 31, 2018
Merged

bpo-25988: Emit a warning when use or import ABCs from 'collections'. #5460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 27 additions & 21 deletions Lib/collections/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,7 @@
__all__ = ['deque', 'defaultdict', 'namedtuple', 'UserDict', 'UserList',
'UserString', 'Counter', 'OrderedDict', 'ChainMap']

# For backwards compatibility, continue to make the collections ABCs
# through Python 3.6 available through the collections module.
# Note, no new collections ABCs were added in Python 3.7
import _collections_abc
from _collections_abc import (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)

from operator import itemgetter as _itemgetter, eq as _eq
from keyword import iskeyword as _iskeyword
import sys as _sys
Expand All @@ -40,30 +31,45 @@
except ImportError:
pass
else:
MutableSequence.register(deque)
_collections_abc.MutableSequence.register(deque)

try:
from _collections import defaultdict
except ImportError:
pass


def __getattr__(name):
# For backwards compatibility, continue to make the collections ABCs
# through Python 3.6 available through the collections module.
# Note, no new collections ABCs were added in Python 3.7
if name in _collections_abc.__all__:
obj = getattr(_collections_abc, name)
import warnings
warnings.warn("Using or importing the ABCs from 'collections' instead "
"of from 'collections.abc' is deprecated, "
"and in 3.8 it will stop working",
DeprecationWarning, stacklevel=2)
globals()[name] = obj
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you store it in global before emitting the warning? Just to avoid any risk of emitting the warning twice. (I'm not sure that it can occur in practice, maybe using multiple threads?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is written in the current form deliberately. If store in global before emitting the warning then if deprecation warnings are converted to errors this could lead to making the name usable without error. I'm not sure this is good.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, I don't know. Maybe add at least a comment just explaining that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add it after beta1. Currently I have a slow connection and don't want to block beta1.

return obj
raise AttributeError(f'module {__name__!r} has no attribute {name!r}')

################################################################################
### OrderedDict
################################################################################

class _OrderedDictKeysView(KeysView):
class _OrderedDictKeysView(_collections_abc.KeysView):

def __reversed__(self):
yield from reversed(self._mapping)

class _OrderedDictItemsView(ItemsView):
class _OrderedDictItemsView(_collections_abc.ItemsView):

def __reversed__(self):
for key in reversed(self._mapping):
yield (key, self._mapping[key])

class _OrderedDictValuesView(ValuesView):
class _OrderedDictValuesView(_collections_abc.ValuesView):

def __reversed__(self):
for key in reversed(self._mapping):
Expand Down Expand Up @@ -215,7 +221,7 @@ def __sizeof__(self):
size += sizeof(self.__root) * n # proxy objects
return size

update = __update = MutableMapping.update
update = __update = _collections_abc.MutableMapping.update

def keys(self):
"D.keys() -> a set-like object providing a view on D's keys"
Expand All @@ -229,7 +235,7 @@ def values(self):
"D.values() -> an object providing a view on D's values"
return _OrderedDictValuesView(self)

__ne__ = MutableMapping.__ne__
__ne__ = _collections_abc.MutableMapping.__ne__

__marker = object()

Expand Down Expand Up @@ -636,7 +642,7 @@ def update(*args, **kwds):
raise TypeError('expected at most 1 arguments, got %d' % len(args))
iterable = args[0] if args else None
if iterable is not None:
if isinstance(iterable, Mapping):
if isinstance(iterable, _collections_abc.Mapping):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change have an impact on performance? If yes, maybe we should keep a _Mapping global?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. In any case the isinstance check is not cheap for ABCs. I suppose that one attribute resolution has insignificant effect on performance.

if self:
self_get = self.get
for elem, count in iterable.items():
Expand Down Expand Up @@ -673,7 +679,7 @@ def subtract(*args, **kwds):
iterable = args[0] if args else None
if iterable is not None:
self_get = self.get
if isinstance(iterable, Mapping):
if isinstance(iterable, _collections_abc.Mapping):
for elem, count in iterable.items():
self[elem] = self_get(elem, 0) - count
else:
Expand Down Expand Up @@ -875,7 +881,7 @@ def __iand__(self, other):
### ChainMap
########################################################################

class ChainMap(MutableMapping):
class ChainMap(_collections_abc.MutableMapping):
''' A ChainMap groups multiple dicts (or other mappings) together
to create a single, updateable view.

Expand Down Expand Up @@ -983,7 +989,7 @@ def clear(self):
### UserDict
################################################################################

class UserDict(MutableMapping):
class UserDict(_collections_abc.MutableMapping):

# Start by filling-out the abstract methods
def __init__(*args, **kwargs):
Expand Down Expand Up @@ -1050,7 +1056,7 @@ def fromkeys(cls, iterable, value=None):
### UserList
################################################################################

class UserList(MutableSequence):
class UserList(_collections_abc.MutableSequence):
"""A more or less complete user-defined wrapper around list objects."""
def __init__(self, initlist=None):
self.data = []
Expand Down Expand Up @@ -1123,7 +1129,7 @@ def extend(self, other):
### UserString
################################################################################

class UserString(Sequence):
class UserString(_collections_abc.Sequence):
def __init__(self, seq):
if isinstance(seq, str):
self.data = seq
Expand Down
6 changes: 5 additions & 1 deletion Modules/_decimal/_decimal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5521,6 +5521,7 @@ PyInit__decimal(void)
PyObject *numbers = NULL;
PyObject *Number = NULL;
PyObject *collections = NULL;
PyObject *collections_abc = NULL;
PyObject *MutableMapping = NULL;
PyObject *obj = NULL;
DecCondMap *cm;
Expand Down Expand Up @@ -5595,7 +5596,8 @@ PyInit__decimal(void)
Py_CLEAR(obj);

/* MutableMapping */
ASSIGN_PTR(MutableMapping, PyObject_GetAttrString(collections,
ASSIGN_PTR(collections_abc, PyImport_ImportModule("collections.abc"));
ASSIGN_PTR(MutableMapping, PyObject_GetAttrString(collections_abc,
"MutableMapping"));
/* Create SignalDict type */
ASSIGN_PTR(PyDecSignalDict_Type,
Expand All @@ -5606,6 +5608,7 @@ PyInit__decimal(void)

/* Done with collections, MutableMapping */
Py_CLEAR(collections);
Py_CLEAR(collections_abc);
Py_CLEAR(MutableMapping);


Expand Down Expand Up @@ -5765,6 +5768,7 @@ PyInit__decimal(void)
Py_CLEAR(Number); /* GCOV_NOT_REACHED */
Py_CLEAR(Rational); /* GCOV_NOT_REACHED */
Py_CLEAR(collections); /* GCOV_NOT_REACHED */
Py_CLEAR(collections_abc); /* GCOV_NOT_REACHED */
Py_CLEAR(MutableMapping); /* GCOV_NOT_REACHED */
Py_CLEAR(SignalTuple); /* GCOV_NOT_REACHED */
Py_CLEAR(DecimalTuple); /* GCOV_NOT_REACHED */
Expand Down