sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.44k stars 480 forks source link

Deprecate sage.misc.misc.union #32096

Closed mkoeppe closed 3 years ago

mkoeppe commented 3 years ago

We have a global binding of union that returns "the union of x and y, as a list". We deprecate it because this is an outdated interface (with strong "early Python" vibes) and not suitable for the global namespace. It is only used in a few places in the Sage library. (There is no intersection.)

Depends on #32140

CC: @tscrim @jhpalmieri

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 699f05b

Reviewer: John Palmieri

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

mkoeppe commented 3 years ago

Branch: u/mkoeppe/deprecate_sage_misc_misc_union

mkoeppe commented 3 years ago
comment:2

There are 3 more uses that I haven't fixed yet


New commits:

00e6c45sage.misc.misc.union: Deprecate, remove uses
mkoeppe commented 3 years ago

Commit: 00e6c45

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 00e6c45 to 6d5ec17

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

cd42cf7src/sage/schemes/elliptic_curves/ell_number_field.py: Remove useless use of union
6d5ec17src/sage/topology/simplicial_complex.py: Remove use of sage.misc.misc.union
mkoeppe commented 3 years ago

Author: Matthias Koeppe

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,3 @@
-We have a global binding of `union` that returns "the union of x and y, as a list". We deprecate it because this is an outdated interface and not suitable for the global namespace. It is only used in a few places in the Sage library. (There is no `intersection`.)
+We have a global binding of `union` that returns "the union of x and y, as a list". We deprecate it because this is an outdated interface (with strong "early Python" vibes) and not suitable for the global namespace. It is only used in a few places in the Sage library. (There is no `intersection`.)
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

7eee760sage.misc.misc.union: Deprecate, remove uses
44da7d8src/sage/schemes/elliptic_curves/ell_number_field.py: Remove useless use of union
8fb8315src/sage/topology/simplicial_complex.py: Remove use of sage.misc.misc.union
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 6d5ec17 to 8fb8315

jhpalmieri commented 3 years ago
comment:6

I'm getting doctest failures because of the deprecation warning:

sage -t --long --warn-long 98.0 --random-seed=0 src/sage/groups/perm_gps/permgroup.py  # 1 doctest failed
sage -t --long --warn-long 98.0 --random-seed=0 src/sage/arith/misc.py  # 1 doctest failed
sage -t --long --warn-long 98.0 --random-seed=0 src/sage/geometry/polyhedron/library.py  # 1 doctest failed
jhpalmieri commented 3 years ago
comment:7

The changes look okay, though.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

bfddaaeRemove uses of sage.misc.misc.union in doctests
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 8fb8315 to bfddaae

mkoeppe commented 3 years ago
comment:9

Thanks for testing! I forgot to look for those

jhpalmieri commented 3 years ago

Reviewer: John Palmieri

jhpalmieri commented 3 years ago
comment:10

Looks good to me.

mkoeppe commented 3 years ago
comment:11

Thanks!

vbraun commented 3 years ago
comment:12
    STDOUT: CONFLICT (content): Merge conflict in src/sage/schemes/elliptic_curves/ell_rational_field.py
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

149aa32Some cleanup of padic documentation.
8e2b0a0Merge branch 'develop' into public/documentation/fix_escape_seq-32140
0be7871Some last little details.
93659dbDoing some additional cleanup and normalization of documentation and errors.
2b2e7bfFixing two other doctest failures in other files from error message changes.
3a30f30Fixing docstrings in ell_rational_field.py.
460e70cSome spelling errors and removed misplaced / character.
699f05bMerge #32140
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from bfddaae to 699f05b

mkoeppe commented 3 years ago

Dependencies: #32140

mkoeppe commented 3 years ago
comment:14

Merged #32140 to resolve merge conflict

vbraun commented 3 years ago

Changed branch from u/mkoeppe/deprecate_sage_misc_misc_union to 699f05b