Repository: wright-group/WrightTools

Issue:
Symmetric square root does not correctly multiple by the sign factor when the output array is supplied
https://github.com/wright-group/WrightTools/blob/e875360573e8375f94ed4ed70a7d8dc02ab92bb5/WrightTools/kit/_calculate.py#L145-L165

Discussion:
deepcode-ci-bot[bot]: <img src="https://www.deepcode.ai/icons/green_check.svg" width= "50px" align= "left"/> Congratulations :beers:. DeepCode [analyzed](https://www.deepcode.ai/app/gh/wright-group/WrightTools/e8966a3807c27c60ec23639601f4db276588d25f/wright-group/WrightTools/940802056e82b25553cb2ff9a8045f32bf35769a/_/%2F/code/?utm_source=gh_review) your code in 0.105 seconds and we found no issues. Enjoy a moment of no bugs :sunny:.

:speech_balloon: This comment has been generated by the [**DeepCode bot**](https://github.com/apps/deepcode-ci-bot/), installed by the owner of the repository. The DeepCode bot **protects** your repository by detecting and commenting on security vulnerabilities or other critical issues.

---
:relaxed: If you want to provide feedback on our bot, here is how to [contact us](https://www.deepcode.ai/feedback?select=4&utm_source=gh_review).

untzag: gosh @ksunden I do NOT understand the bug---can you explain it to me?
ksunden: Wihout this change:

```
>>> a = np.linspace(-9, 9, 10)
>>> a
array([-9., -7., -5., -3., -1.,  1.,  3.,  5.,  7.,  9.])
>>> wt.kit.symmetric_sqrt(a)
array([-3.        , -2.64575131, -2.23606798, -1.73205081, -1.        ,
        1.        ,  1.73205081,  2.23606798,  2.64575131,  3.        ])
>>> o = np.empty_like(a)
>>> wt.kit.symmetric_sqrt(a, out=o)
array([-3.        , -2.64575131, -2.23606798, -1.73205081, -1.        ,
        1.        ,  1.73205081,  2.23606798,  2.64575131,  3.        ])
>>> o
array([3.        , 2.64575131, 2.23606798, 1.73205081, 1.        ,
       1.        , 1.73205081, 2.23606798, 2.64575131, 3.        ])
>>>
```
With this change:
```

>>> a = np.linspace(-9, 9, 10)
>>> a
array([-9., -7., -5., -3., -1.,  1.,  3.,  5.,  7.,  9.])
>>> wt.kit.symmetric_sqrt(a)
array([-3.        , -2.64575131, -2.23606798, -1.73205081, -1.        ,
        1.        ,  1.73205081,  2.23606798,  2.64575131,  3.        ])
>>> o = np.empty_like(a)
>>> wt.kit.symmetric_sqrt(a, out=o)
array([-3.        , -2.64575131, -2.23606798, -1.73205081, -1.        ,
        1.        ,  1.73205081,  2.23606798,  2.64575131,  3.        ])
>>> o
array([-3.        , -2.64575131, -2.23606798, -1.73205081, -1.        ,
        1.        ,  1.73205081,  2.23606798,  2.64575131,  3.        ])
>>>
```

Note the difference in the sign of the negative valued inputs.

The `out` parameter is meant to avoid reallocating the output array (As it does in numpy functions)

Without this change, the returned array is a reallocated array (by the `*` operator), and the provided output array is _not_ multiplied by the sign.

I can add a test if desired, this was mostly me finding a small change to walk through an example of opening up a pull request with @kameyer226 watching (but it does have an associated issue)
untzag: @ksunden I think adding a test would be nice, but feel free to merge if you wish
darienmorrow: @ksunden pretty sure that test isn't gonna pass. sqrt(9) != 0
ksunden: @darienmorrow, this is symmetric sqrt, not normal sqrt, the whole point being that negative numbers have negative output, I did in fact run the test locally and it did pass
darienmorrow: @ksunden I guess I don't understand the test, then. I thought the test was asserting that sym_sqrt([-9, 9_[sic]_, 3]) = [-3, 0, 3] which just isn't true even considering float vs. int map.
ksunden: @darienmorrow I initially misread your comment, I updated to reflect what you actually said, sorry for the confusion.

It is not asserting that `symmeric_sqrt([-9,9,3]) == [-3, 0, 3]`, it is asserting that `symmeric_sqrt([-9,0,9]) == [-3, 0, 3]`
darienmorrow: @ksunden I see my problem! 😬 I read `np.array([-9,9,3])` when your test actually has `np.linspace(-9, 9, 3)`. Gosh this is embarrassing.
ksunden: No worries, all good now

Patch:
diff --git a/WrightTools/kit/_calculate.py b/WrightTools/kit/_calculate.py
index 50adda6..2aaa11e 100644
--- a/WrightTools/kit/_calculate.py
+++ b/WrightTools/kit/_calculate.py
@@ -137,8 +137,8 @@ def nm_width(center, width, units="wn") -> float:
     number
         Width in nm.
     """
-    red = wt_units.converter(center - width / 2., units, "nm")
-    blue = wt_units.converter(center + width / 2., units, "nm")
+    red = wt_units.converter(center - width / 2.0, units, "nm")
+    blue = wt_units.converter(center + width / 2.0, units, "nm")
     return red - blue
 
 
@@ -162,4 +162,5 @@ def symmetric_sqrt(x, out=None):
     """
     factor = np.sign(x)
     out = np.sqrt(np.abs(x), out=out)
-    return out * factor
+    out *= factor
+    return out
diff --git a/WrightTools/kit/_interpolate.py b/WrightTools/kit/_interpolate.py
index aaf5438..86667ea 100644
--- a/WrightTools/kit/_interpolate.py
+++ b/WrightTools/kit/_interpolate.py
@@ -21,7 +21,7 @@ __all__ = ["zoom2D", "Spline"]
 # --- functions -----------------------------------------------------------------------------------
 
 
-def zoom2D(xi, yi, zi, xi_zoom=3., yi_zoom=3., order=3, mode="nearest", cval=0.):
+def zoom2D(xi, yi, zi, xi_zoom=3.0, yi_zoom=3.0, order=3, mode="nearest", cval=0.0):
     """Zoom a 2D array, with axes.
 
     Parameters
diff --git a/WrightTools/kit/_timestamp.py b/WrightTools/kit/_timestamp.py
index 6ef2355..8a9c01e 100644
--- a/WrightTools/kit/_timestamp.py
+++ b/WrightTools/kit/_timestamp.py
@@ -159,7 +159,7 @@ class TimeStamp:
         format_string = "%Y-%m-%dT%H:%M:%S.%f"
         out = self.datetime.strftime(format_string)
         # timezone
-        if delta_sec == 0.:
+        if delta_sec == 0.0:
             out += "Z"
         else:
             if delta_sec > 0:


Test patch:
diff --git a/tests/kit/symmetric_sqrt.py b/tests/kit/symmetric_sqrt.py
index 2cf5c93..55112d0 100644
--- a/tests/kit/symmetric_sqrt.py
+++ b/tests/kit/symmetric_sqrt.py
@@ -17,3 +17,11 @@ def test_numbers():
     for number in numbers:
         answer = wt.kit.symmetric_sqrt(number)
         assert answer == np.sign(number) * np.sqrt(np.abs(number))
+
+
+def test_no_reallocation():
+    a = np.linspace(-9, 9, 3)
+    out = np.empty_like(a)
+    ret = wt.kit.symmetric_sqrt(a, out=out)
+    assert out is ret
+    assert np.allclose(ret, [-3, 0, 3])