Skip to content

Fix DeprecationWarning from classes in collections.abc #1134

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

Merged
merged 1 commit into from
Nov 23, 2018
Merged

Fix DeprecationWarning from classes in collections.abc #1134

merged 1 commit into from
Nov 23, 2018

Conversation

yan12125
Copy link
Contributor

@yan12125 yan12125 commented Nov 21, 2018

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


This change is Reviewable

Copy link
Collaborator

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. :lgtm:

For anyone unable to get the same warning locally, here's how:

  • In ycmd root open python -Wa
  • from ycmd.utils import HashableDict

I love how python is trying to hide away the deprecation warning...

Reviewed 1 of 1 files at r1, 1 of 1 files at r2.
Reviewable status: 1 of 2 LGTMs obtained

Copy link
Contributor Author

@yan12125 yan12125 left a comment

Choose a reason for hiding this comment

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

Here a simpler way: export PYTHONWARNINGS=all and run vim!

Reviewable status: 1 of 2 LGTMs obtained

@codecov
Copy link

codecov bot commented Nov 21, 2018

Codecov Report

Merging #1134 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1134      +/-   ##
==========================================
+ Coverage   97.67%   97.67%   +<.01%     
==========================================
  Files          90       90              
  Lines        7067     7068       +1     
==========================================
+ Hits         6903     6904       +1     
  Misses        164      164

Copy link
Collaborator

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 2 LGTMs obtained


ycmd/utils.py, line 31 at r2 (raw file):

  from collections.abc import Mapping
except ImportError: # pragma: no cover
  # Python < 3.3

Let's mark this Python 2, or something like that, since ycmd doesn't support python 3.0 to 3.3 and the only way to get here is running python 2.7.
It will be easier to spot once we drop python 2.7 as well.

Copy link
Collaborator

@micbou micbou left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

Reviewed 1 of 1 files at r2.
Reviewable status: 1 of 2 LGTMs obtained


ycmd/utils.py, line 32 at r2 (raw file):

except ImportError: # pragma: no cover
  # Python < 3.3
  from collections import Mapping # noqa

I would move this code in the if PY2 block further below.

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
Copy link
Contributor Author

@yan12125 yan12125 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 LGTMs obtained (and 1 stale)


ycmd/utils.py, line 32 at r2 (raw file):

Previously, micbou wrote…

I would move this code in the if PY2 block further below.

Should be fixed now :)

Copy link
Collaborator

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

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

:lgtm_strong:

Reviewed 1 of 1 files at r3.
Reviewable status: 1 of 2 LGTMs obtained

Copy link
Collaborator

@micbou micbou left a comment

Choose a reason for hiding this comment

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

:lgtm_strong:

Reviewed 1 of 1 files at r3.
Reviewable status: :shipit: complete! 2 of 2 LGTMs obtained

@micbou
Copy link
Collaborator

micbou commented Nov 23, 2018

@zzbot r+

@zzbot
Copy link
Contributor

zzbot commented Nov 23, 2018

📌 Commit 7089fec has been approved by micbou

@zzbot
Copy link
Contributor

zzbot commented Nov 23, 2018

⌛ Testing commit 7089fec with merge b9e9430...

zzbot added a commit 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 -->
@zzbot
Copy link
Contributor

zzbot commented Nov 23, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: micbou
Pushing b9e9430 to master...

@zzbot zzbot merged commit 7089fec into ycm-core:master Nov 23, 2018
@yan12125 yan12125 deleted the fix-collections-abc-warning branch November 24, 2018 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants