sagemath/sage

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

Closed this issue · 14 comments

$ 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

Description changed:

--- 
+++ 
@@ -2,3 +2,4 @@
 
 This will avoid the dependency on `sage.symbolic`
 
+Same for `ceil`, `floor`, but that that could also be done via #25827.

Description changed:

--- 
+++ 
@@ -2,4 +2,4 @@
 
 This will avoid the dependency on `sage.symbolic`
 
-Same for `ceil`, `floor`, but that that could also be done via #25827.
+Same for `ceil`, `floor`, but that could also be done via #25827.

Description changed:

--- 
+++ 
@@ -1,3 +1,6 @@
+`$ git grep 'functions.*import.*sqrt'` finds a small number of these imports
+
+
 It can be replaced by a call to the `.sqrt` method
 
 This will avoid the dependency on `sage.symbolic`

Description changed:

--- 
+++ 
@@ -1,8 +1,9 @@
 `$ 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`. `isqrt` already lives there.
 
-It can be replaced by a call to the `.sqrt` method
+Alternatively, the uses of the `sqrt` function can be replaced by a call to the `.sqrt` method.
 
-This will avoid the dependency on `sage.symbolic`
+Either way will avoid the dependency on `sage.symbolic`.
 
-Same for `ceil`, `floor`, but that could also be done via #25827.
+(Same for `ceil`, `floor`, but that could also be done via #25827.)

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

Author: Matthias Koeppe

Description changed:

--- 
+++ 
@@ -1,8 +1,8 @@
 `$ 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`. `isqrt` already lives there.
+`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, the uses of the `sqrt` function can be replaced by a call to the `.sqrt` method.
+(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`.
 
comment:8

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

comment:10

LGTM.

Reviewer: Travis Scrimshaw

comment:11

Thanks!