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

Move sage.functions.other.sqrt to sage.misc.functional #32717

Closed
mkoeppe opened this issue Oct 18, 2021 · 14 comments
Closed

Move sage.functions.other.sqrt to sage.misc.functional #32717

mkoeppe opened this issue Oct 18, 2021 · 14 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 18, 2021

$ git grep 'functions.*import.*sqrt' finds a small number of these imports

sqrt is actually not a symbolic function, so we move it to sage.misc.functional and import it from there. isqrt already lives there.

(Alternatively, many uses of the sqrt function could be replaced by a call to the .sqrt method.)

Either way will avoid the dependency on sage.symbolic.

(Same for ceil, floor, but that could also be done via #25827.)

CC: @tscrim

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 451ac27

Reviewer: Travis Scrimshaw

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

@mkoeppe mkoeppe added this to the sage-9.5 milestone Oct 18, 2021
@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 20, 2021

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 20, 2021

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

5952b37src/sage/misc/all.py: Add sqrt
905e800src/sage/misc/functional.py: Fix imports
451ac27src/sage/functions/other.py: Remove import from sage.rings.all

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 20, 2021

Commit: 451ac27

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 20, 2021

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 20, 2021

comment:8

The failure in src/sage/rings/integer.pyx is not from this ticket

@mkoeppe mkoeppe changed the title Remove use of sage.functions.other.sqrt (outside of sage.symbolic, sage.functions, sage.manifolds) Move sage.functions.other.sqrt to sage.misc.functional Oct 20, 2021
@tscrim
Copy link
Collaborator

tscrim commented Oct 21, 2021

comment:10

LGTM.

@tscrim
Copy link
Collaborator

tscrim commented Oct 21, 2021

Reviewer: Travis Scrimshaw

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 21, 2021

comment:11

Thanks!

@vbraun
Copy link
Member

vbraun commented Oct 24, 2021

@vbraun vbraun closed this as completed in d8e6aed Oct 24, 2021
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 17, 2023
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