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.

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

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

Merged

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jan 31, 2018

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Looks good to me!

"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.

@@ -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.

@serhiy-storchaka serhiy-storchaka merged commit c66f9f8 into python:master Jan 31, 2018
@bedevere-bot
Copy link

@serhiy-storchaka: Please replace # with GH- in the commit message next time. Thanks!

@serhiy-storchaka serhiy-storchaka deleted the collections-abc-deprecations branch January 31, 2018 17:20
ilevkivskyi pushed a commit to ilevkivskyi/cpython that referenced this pull request Feb 18, 2018
JoseKilo added a commit to JoseKilo/aiozmq that referenced this pull request Sep 27, 2018
JoseKilo added a commit to JoseKilo/aiozmq that referenced this pull request Sep 27, 2018
zzbot added a commit to ycm-core/ycmd that referenced this pull request Nov 23, 2018
Fix DeprecationWarning from classes in collections.abc

This patch removes the following warning:
```
/usr/share/nvim/runtime/third_party/ycmd/ycmd/utils.py:499: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working
  class HashableDict( collections.Mapping ):
```
This warning is originated from python/cpython#5460

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/1134)
<!-- Reviewable:end -->
openstack-mirroring pushed a commit to openstack/trove that referenced this pull request Mar 20, 2022
This PR stops using the following deprecated functions in std Python lib.

* parser.readfp
  We should use parser.read_file(readline_generator(fp))[1].
* importing modules from collections directly.
  We should use collections.abc instead of using collections directly.[2]

--
[1]: https://docs.python.org/3/library/configparser.html#configparser.ConfigParser.readfp
[2]: python/cpython#5460

Task: 44767
Story: 2009917
Change-Id: I61bf4299ad2acd8ee26b4aab66875b10287020e1
openstack-mirroring pushed a commit to openstack/openstack that referenced this pull request Mar 20, 2022
* Update trove from branch 'master'
  to 04e85c1be0732b7f9333468723b1e1b26d6ddc02
  - Merge "Stop using deprecated functions in std Python lib"
  - Stop using deprecated functions in std Python lib
    
    This PR stops using the following deprecated functions in std Python lib.
    
    * parser.readfp
      We should use parser.read_file(readline_generator(fp))[1].
    * importing modules from collections directly.
      We should use collections.abc instead of using collections directly.[2]
    
    --
    [1]: https://docs.python.org/3/library/configparser.html#configparser.ConfigParser.readfp
    [2]: python/cpython#5460
    
    Task: 44767
    Story: 2009917
    Change-Id: I61bf4299ad2acd8ee26b4aab66875b10287020e1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants