Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.
Sign upbpo-29620: iterate over a tuple of sys.modules #4800
Conversation
the-knights-who-say-ni
added
the
CLA signed
label
Dec 11, 2017
bedevere-bot
added
the
awaiting review
label
Dec 11, 2017
serhiy-storchaka
reviewed
Aug 21, 2018
|
Could you please add a test? It is enough to create a Add also a news entry (see https://devguide.python.org/committing/#what-s-new-and-news-entries). |
| @@ -227,7 +227,7 @@ class _AssertWarnsContext(_AssertRaisesBaseContext): | |||
| def __enter__(self): | |||
| # The __warningregistry__'s need to be in a pristine state for tests | |||
| # to work properly. | |||
| for v in sys.modules.values(): | |||
| for v in tuple(sys.modules.values()): | |||
This comment has been minimized.
This comment has been minimized.
serhiy-storchaka
Aug 21, 2018
Member
Use list(). There is no benefit of using a tuple, but creating a list from a general iterable is a tiny bit faster.
tuple(x) is expanded to tuple(list(x)) in most cases (if x is not a tuple or list).
kernc
force-pushed the
kernc:patch-1
branch
from
2fe6671
to
5651335
Aug 21, 2018
This comment has been minimized.
This comment has been minimized.
|
Done. Hope it's ok. |
serhiy-storchaka
reviewed
Aug 21, 2018
| sys.modules['@bar@'] = 'bar' | ||
|
|
||
| sys.modules['@foo@'] = Foo('foo') | ||
| self.assertWarns(Warning, lambda: warnings.warn('Ok')) |
This comment has been minimized.
This comment has been minimized.
serhiy-storchaka
Aug 21, 2018
Member
I prefer either
self.assertWarns(UserWarning, warnings.warn, 'Ok')or
with self.assertWarns(UserWarning):
warnings.warn('Ok')|
|
||
| sys.modules['@foo@'] = Foo('foo') | ||
| self.assertWarns(Warning, lambda: warnings.warn('Ok')) | ||
| del sys.modules['@foo@'], \ |
This comment has been minimized.
This comment has been minimized.
| @@ -0,0 +1,2 @@ | |||
| In :func:`~unittest.TestCase.assertWarns`, iterate over a copy of | |||
This comment has been minimized.
This comment has been minimized.
serhiy-storchaka
Aug 21, 2018
Member
Making a copy for iteration is an implementation detail. The user visible change is that assertWarns() no longer raises a RuntimeException when __warningregistry__ causes importing a module or a module is imported in other thread.
| @@ -0,0 +1,2 @@ | |||
| In :func:`~unittest.TestCase.assertWarns`, iterate over a copy of | |||
| ``sys.modules``, which may change size during iteration. | |||
This comment has been minimized.
This comment has been minimized.
kernc
force-pushed the
kernc:patch-1
branch
from
5651335
to
7f7fdac
Aug 22, 2018
This comment has been minimized.
This comment has been minimized.
|
Updated. Thanks. |
This comment has been minimized.
This comment has been minimized.
|
@serhiy-storchaka Do you find any other issues? |
kernc commentedDec 11, 2017
•
edited by bedevere-bot
Fixes https://bugs.python.org/issue29620 by wrapping
sys.modules.values()into a tuple before iteration.The change is so miniscule I was not sure it warrants tests.
https://bugs.python.org/issue29620