Opened 2 years ago

Closed 15 months ago

#30768 closed task (fixed)

Restore deprecation warnings for imports from collections vs collections.abc

Reported by: Samuel Lelièvre Owned by:
Priority: major Milestone: sage-9.5
Component: refactoring Keywords:
Cc: Merged in:
Authors: Frédéric Chapoton Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: edc435e (Commits, GitHub, GitLab) Commit: edc435e122cd35abc280faad1ba0dca1fd107105
Dependencies: #32372 Stopgaps:

Status badges

Description (last modified by Samuel Lelièvre)

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 ticket:28012#comment:23 for the discussion.

Change History (16)

comment:1 Changed 2 years ago by Samuel Lelièvre

The required change is:

  • src/sage/all.py

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

comment:2 Changed 2 years ago by Frédéric Chapoton

Branch: u/chapoton/30768
Commit: d4a64b35925a4a74e79e68354b55711b749b3d0e

voila une branche


New commits:

d4a64b3re-activate abc warnings

comment:3 Changed 16 months ago by Frédéric Chapoton

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

Last edited 16 months ago by Frédéric Chapoton (previous) (diff)

comment:4 Changed 16 months ago by Matthias Köppe

Dependencies: #32372

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

comment:5 Changed 16 months ago by Matthias Köppe

Branch: u/chapoton/30768u/mkoeppe/30768

comment:6 Changed 16 months ago by Matthias Köppe

Commit: d4a64b35925a4a74e79e68354b55711b749b3d0e5ec278c528ed0390613135c229a72dd4e3addc73

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

comment:7 Changed 16 months ago by Matthias Köppe

Authors: Frédéric Chapoton

comment:8 Changed 16 months ago by Matthias Köppe

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

comment:9 Changed 16 months ago by Matthias Köppe

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

comment:10 Changed 16 months ago by git

Commit: 5ec278c528ed0390613135c229a72dd4e3addc73edc435e122cd35abc280faad1ba0dca1fd107105

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

comment:11 Changed 15 months ago by Samuel Lelièvre

Description: modified (diff)

Is this ready for review?

comment:12 Changed 15 months ago by Frédéric Chapoton

yes, but depending on #32372

comment:13 Changed 15 months ago by Frédéric Chapoton

Status: newneeds_review

now this should be ready to go

comment:14 Changed 15 months ago by Matthias Köppe

Milestone: sage-pendingsage-9.5

comment:15 Changed 15 months ago by Matthias Köppe

Reviewers: Matthias Koeppe
Status: needs_reviewpositive_review

comment:16 Changed 15 months ago by Volker Braun

Branch: u/mkoeppe/30768edc435e122cd35abc280faad1ba0dca1fd107105
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.