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

Restore deprecation warnings for imports from collections vs collections.abc #30768

Closed
slel opened this issue Oct 13, 2020 · 21 comments
Closed

Comments

@slel
Copy link
Member

slel commented Oct 13, 2020

Since #28012 we correctly import "collections
abstract base classes" from collections.abc.

This ticket is for the next step: unsilence the
corresponding deprecation warnings.

We postponed doing that since some of our dependencies
had not updated their imports yet.

See #28012 comment:23 for the discussion.

Depends on #32372

Component: refactoring

Author: Frédéric Chapoton

Branch/Commit: edc435e

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/30768

@slel
Copy link
Member Author

slel commented Oct 13, 2020

comment:1

The required change is:

diff --git a/src/sage/all.py b/src/sage/all.py
--- a/src/sage/all.py
+++ b/src/sage/all.py
@@ -306,10 +306,6 @@ warnings.filters.remove(('ignore', None, DeprecationWarning, None, 0))
 # Ignore all deprecations from IPython etc.
 warnings.filterwarnings('ignore', category=DeprecationWarning,
     module='.*(IPython|ipykernel|jupyter_client|jupyter_core|nbformat|notebook|ipywidgets|storemagic)')
-# Ignore collections.abc warnings, there are a lot of them but they are
-# harmless.
-warnings.filterwarnings('ignore', category=DeprecationWarning,
-    message='.*collections[.]abc.*')
 # However, be sure to keep OUR deprecation warnings
 warnings.filterwarnings('default', category=DeprecationWarning,
     message=r'[\s\S]*See https\?://trac.sagemath.org/[0-9]* for details.')

@fchapoton
Copy link
Contributor

Commit: d4a64b3

@fchapoton
Copy link
Contributor

comment:2

voila une branche


New commits:

d4a64b3re-activate abc warnings

@fchapoton
Copy link
Contributor

Branch: u/chapoton/30768

@fchapoton
Copy link
Contributor

comment:3

urllib3 seems to be the main source of problems

EDIT:

but this seems to comes from urllib3 from inside requests

updating requests seem to fix a lot of them

requests should be upgraded in #31280

EDIT:

updating request apparently fixes all doctests

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 12, 2021

Dependencies: #32372

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 12, 2021

comment:4

The update of requests (with unvendored urllib3) is now split out to #32372

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 12, 2021

Changed branch from u/chapoton/30768 to u/mkoeppe/30768

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 12, 2021

Changed commit from d4a64b3 to 5ec278c

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 12, 2021

comment:6

Rebased on top of #32372


New commits:

764b341build/pkgs/chardet: New (unvendored from requests)
5b5e721build/pkgs/idna: New (unvendored from requests)
649f5afbuild/pkgs/urllib3: New (unvendored from requests)
b8cf517build/pkgs/requests: Update to 2.26.0
b5dc185build/pkgs/requests/dependencies: Add unvendored packages
9a05776build/pkgs/urllib3: Update to 1.26.6
fbb7da9build/pkgs/idna: Update to 3.2
81af3bfbuild/pkgs/requests/checksums.ini: Add upstream_url
5ec278cre-activate abc warnings

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 12, 2021

Author: Frédéric Chapoton

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 12, 2021

comment:8

Have you tracked down what versions of urllib3 / requests did the fix? We will need to put corresponding version lower bounds into build/pkgs/urllib3/install-requires.txt and build/pkgs/requests/install-requires.txt

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 13, 2021

comment:9

According to https://docs.python-requests.org/en/master/community/updates/#id11, requests 2.19.0 did "Migrate to using collections.abc for 3.7 compatibility"; 2.16.0 did the unvendoring of urllib3 etc.
requests main branch sets the constraint urllib3>=1.21.1,<1.27 in https://github.com/psf/requests/blob/main/setup.py; the lower bound has not changed since 2.16.0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 13, 2021

Changed commit from 5ec278c to edc435e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 13, 2021

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8c8a766build/pkgs/charset_normalizer: New, use it instead of chardet as a dependency of requests
8cc62debuild/pkgs/chardet: Remove
eced0deMerge #32372
edc435ere-activate abc warnings

@slel

This comment has been minimized.

@slel
Copy link
Member Author

slel commented Aug 29, 2021

comment:11

Is this ready for review?

@fchapoton
Copy link
Contributor

comment:12

yes, but depending on #32372

@fchapoton
Copy link
Contributor

comment:13

now this should be ready to go

@mkoeppe mkoeppe added this to the sage-9.5 milestone Sep 15, 2021
@mkoeppe mkoeppe removed the pending label Sep 15, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 15, 2021

Reviewer: Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented Sep 19, 2021

Changed branch from u/mkoeppe/30768 to edc435e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants