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

Deprecate sage.misc.misc.union #32096

Closed
mkoeppe opened this issue Jul 1, 2021 · 23 comments
Closed

Deprecate sage.misc.misc.union #32096

mkoeppe opened this issue Jul 1, 2021 · 23 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 1, 2021

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 mkoeppe added this to the sage-9.4 milestone Jul 1, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 2, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 2, 2021

comment:2

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


New commits:

00e6c45sage.misc.misc.union: Deprecate, remove uses

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 2, 2021

Commit: 00e6c45

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 2, 2021

Changed commit from 00e6c45 to 6d5ec17

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 2, 2021

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

mkoeppe commented Jul 2, 2021

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 2, 2021

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 2, 2021

Changed commit from 6d5ec17 to 8fb8315

@jhpalmieri
Copy link
Member

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
Copy link
Member

comment:7

The changes look okay, though.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 3, 2021

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

bfddaaeRemove uses of sage.misc.misc.union in doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 3, 2021

Changed commit from 8fb8315 to bfddaae

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 3, 2021

comment:9

Thanks for testing! I forgot to look for those

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:10

Looks good to me.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 5, 2021

comment:11

Thanks!

@vbraun
Copy link
Member

vbraun commented Jul 9, 2021

comment:12
    STDOUT: CONFLICT (content): Merge conflict in src/sage/schemes/elliptic_curves/ell_rational_field.py

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 9, 2021

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 9, 2021

Changed commit from bfddaae to 699f05b

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 9, 2021

Dependencies: #32140

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 9, 2021

comment:14

Merged #32140 to resolve merge conflict

@vbraun
Copy link
Member

vbraun commented Jul 23, 2021

Changed branch from u/mkoeppe/deprecate_sage_misc_misc_union to 699f05b

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

3 participants