Repository: PlasmaPy/PlasmaPy

Issue:
Add units to Magnetostatics reprs
Objects from the Magnetostatics module currently show up in reprs without units, because they're being cast to floats on class construction. It would be nice to have them save the input units, or at the very least, to display the units they're saving their inputs to, in their reprs and strs. 

Discussion:
pep8speaks: Hello @Ishinomori! Thanks for submitting your pull request.

* In the file [`plasmapy/formulary/magnetostatics.py`](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py):

> [Line 128:13](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py#L128): [E122](https://duckduckgo.com/?q=pep8%20E122) continuation line missing indentation or outdented
> [Line 129:13](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py#L129): [E122](https://duckduckgo.com/?q=pep8%20E122) continuation line missing indentation or outdented
> [Line 130:13](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py#L130): [E122](https://duckduckgo.com/?q=pep8%20E122) continuation line missing indentation or outdented
> [Line 131:13](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py#L131): [E122](https://duckduckgo.com/?q=pep8%20E122) continuation line missing indentation or outdented
> [Line 132:13](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py#L132): [E122](https://duckduckgo.com/?q=pep8%20E122) continuation line missing indentation or outdented
> [Line 133:13](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py#L133): [E122](https://duckduckgo.com/?q=pep8%20E122) continuation line missing indentation or outdented
> [Line 134:9](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py#L134): [E122](https://duckduckgo.com/?q=pep8%20E122) continuation line missing indentation or outdented
> [Line 401:13](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py#L401): [E122](https://duckduckgo.com/?q=pep8%20E122) continuation line missing indentation or outdented
> [Line 402:13](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py#L402): [E122](https://duckduckgo.com/?q=pep8%20E122) continuation line missing indentation or outdented
> [Line 403:13](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py#L403): [E122](https://duckduckgo.com/?q=pep8%20E122) continuation line missing indentation or outdented
> [Line 404:13](https://github.com/PlasmaPy/PlasmaPy/blob/5965a100cad5430ddd2af9cf228c5e3cb06d298d/plasmapy/formulary/magnetostatics.py#L404): [E122](https://duckduckgo.com/?q=pep8%20E122) continuation line missing indentation or outdented

PlasmaPy follows the [PEP8 style guide](https://www.python.org/dev/peps/pep-0008/), which may be checked locally using [pycodestyle](http://pycodestyle.pycqa.org/en/latest/intro.html)
StanczakDominik: LGTM, won't break existing calculations nor changed ones from ParticleTracker PR.

One optional things to consider: the reprs code could be made shorter by using f-strings. Optional, though!

Would review as approve but phone app is crashing
Ishinomori: Going to adjust all the test `test_repr`. There is also an issue that `CompositeUnit` object has no attribute 'get_format_name'. But I have no idea whats wrong with pep8 unconformity.
Ishinomori: > LGTM, won't break existing calculations nor changed ones from ParticleTracker PR.
> 
> One optional things to consider: the reprs code could be made shorter by using f-strings. Optional, though!
> 
> Would review as approve but phone app is crashing

Am also fan o `fstrings` but was not sure if py35 is really supported.
StanczakDominik: Nope, all out py36+ here! 
StanczakDominik: With respect to pep8, indents are either too short or too long, hard to tell from phone. Try the autopep8 tool or black, that should shut the bot up. 
codecov[bot]: # [Codecov](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743?src=pr&el=h1) Report
> Merging [#743](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743?src=pr&el=desc) into [master](https://codecov.io/gh/PlasmaPy/PlasmaPy/commit/46a77797ff64d61d4215f5b686687359f892d9dd?src=pr&el=desc) will **decrease** coverage by `0.24%`.
> The diff coverage is `88.23%`.

[![Impacted file tree graph](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743/graphs/tree.svg?width=650&token=LzZ5mnXVad&height=150&src=pr)](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #743      +/-   ##
==========================================
- Coverage   95.06%   94.82%   -0.25%     
==========================================
  Files          56       56              
  Lines        5047     5025      -22     
==========================================
- Hits         4798     4765      -33     
- Misses        249      260      +11
```


| [Impacted Files](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [plasmapy/formulary/magnetostatics.py](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743/diff?src=pr&el=tree#diff-cGxhc21hcHkvZm9ybXVsYXJ5L21hZ25ldG9zdGF0aWNzLnB5) | `95% <88.23%> (-4.22%)` | :arrow_down: |
| [plasmapy/classes/sources/plasma3d.py](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743/diff?src=pr&el=tree#diff-cGxhc21hcHkvY2xhc3Nlcy9zb3VyY2VzL3BsYXNtYTNkLnB5) | `88.09% <0%> (-11.91%)` | :arrow_down: |
| [plasmapy/classes/plasma\_base.py](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743/diff?src=pr&el=tree#diff-cGxhc21hcHkvY2xhc3Nlcy9wbGFzbWFfYmFzZS5weQ==) | `67.74% <0%> (-4.49%)` | :arrow_down: |
| [plasmapy/classes/sources/openpmd\_hdf5.py](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743/diff?src=pr&el=tree#diff-cGxhc21hcHkvY2xhc3Nlcy9zb3VyY2VzL29wZW5wbWRfaGRmNS5weQ==) | `80.23% <0%> (-1.09%)` | :arrow_down: |
| [plasmapy/simulation/particletracker.py](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743/diff?src=pr&el=tree#diff-cGxhc21hcHkvc2ltdWxhdGlvbi9wYXJ0aWNsZXRyYWNrZXIucHk=) | `98.27% <0%> (-0.03%)` | :arrow_down: |
| [plasmapy/atomic/ionization\_state.py](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743/diff?src=pr&el=tree#diff-cGxhc21hcHkvYXRvbWljL2lvbml6YXRpb25fc3RhdGUucHk=) | `92.91% <0%> (-0.03%)` | :arrow_down: |
| [plasmapy/utils/pytest\_helpers/pytest\_helpers.py](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743/diff?src=pr&el=tree#diff-cGxhc21hcHkvdXRpbHMvcHl0ZXN0X2hlbHBlcnMvcHl0ZXN0X2hlbHBlcnMucHk=) | `93.38% <0%> (-0.03%)` | :arrow_down: |
| [plasmapy/atomic/particle\_class.py](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743/diff?src=pr&el=tree#diff-cGxhc21hcHkvYXRvbWljL3BhcnRpY2xlX2NsYXNzLnB5) | `96.63% <0%> (-0.01%)` | :arrow_down: |
| [plasmapy/formulary/braginskii.py](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743/diff?src=pr&el=tree#diff-cGxhc21hcHkvZm9ybXVsYXJ5L2JyYWdpbnNraWkucHk=) | `99.72% <0%> (-0.01%)` | :arrow_down: |
| ... and [5 more](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743?src=pr&el=footer). Last update [46a7779...69f6e2c](https://codecov.io/gh/PlasmaPy/PlasmaPy/pull/743?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Ishinomori: What is case of `CompositeUnit` which is dimensionless? Should we have isinstance check on attribute assignment or something else?
StanczakDominik: Tests are failing, could you look into why?
Ishinomori: > Tests are failing, could you look into why?

Sure. Starting to work on it.
Ishinomori: Not sure if should I add number of issue or PR to changelog?
Ishinomori: It's ready for review.

Patch:
diff --git a/changelog/743.feature.rst b/changelog/743.feature.rst
new file mode 100644
index 00000000..52ebf1bc
--- /dev/null
+++ b/changelog/743.feature.rst
@@ -0,0 +1,1 @@
+Added units to reprs of .formulary.magnetostatics classes.
\ No newline at end of file
diff --git a/docs/about/credits.rst b/docs/about/credits.rst
index 22d12556..d008e528 100644
--- a/docs/about/credits.rst
+++ b/docs/about/credits.rst
@@ -78,6 +78,7 @@ in parentheses are `ORCID author identifiers <https://orcid.org>`__.
 * `Thomas Varnish <https://github.com/tvarnish>`__
 * `Aditya Magarde <https://github.com/adityamagarde>`__
 * `Diego A. Diaz Riega <https://github.com/diego7319>`__
+* `Jakub Polak <https://github.com/Ishinomori>`__
 
 This list contains contributors to PlasmaPy's core package and vision
 statement, including a few people who do not show up as `PlasmaPy
diff --git a/plasmapy/formulary/magnetostatics.py b/plasmapy/formulary/magnetostatics.py
index 29eb09ae..2a2de9f0 100644
--- a/plasmapy/formulary/magnetostatics.py
+++ b/plasmapy/formulary/magnetostatics.py
@@ -46,16 +46,21 @@ class MagneticDipole(MagnetoStatics):
         Position of the dipole
 
     """
+
     @validate_quantities
     def __init__(self, moment: u.A * u.m**2, p0: u.m):
         self.moment = moment.value
+        self._moment_u = moment.unit
         self.p0 = p0.value
+        self._p0_u = p0.unit
 
     def __repr__(self):
-        return "{name}(moment={moment}, p0={p0})".format(
+        return "{name}(moment={moment}{moment_u}, p0={p0}{p0_u})".format(
             name=self.__class__.__name__,
             moment=self.moment,
-            p0=self.p0
+            p0=self.p0,
+            moment_u=self._moment_u,
+            p0_u=self._p0_u,
         )
 
     def magnetic_field(self, p: u.m) -> u.T:
@@ -101,11 +106,9 @@ class GeneralWire(Wire):
         electric current
 
     """
+
     @validate_quantities
-    def __init__(self, parametric_eq,
-                 t1,
-                 t2,
-                 current: u.A):
+    def __init__(self, parametric_eq, t1, t2, current: u.A):
         if callable(parametric_eq):
             self.parametric_eq = parametric_eq
         else:
@@ -116,6 +119,20 @@ def __init__(self, parametric_eq,
         else:
             raise ValueError(f"t1={t1} is not smaller than t2={t2}")
         self.current = current.value
+        self._current_u = current.unit
+
+    def __repr__(self):
+        return (
+            "{name}(parametric_eq={parametric_eq}, t1={t1}, t2={t2}, "
+            "current={current}{current_u})".format(
+                name=self.__class__.__name__,
+                parametric_eq=self.parametric_eq.__name__,
+                t1=self.t1,
+                t2=self.t2,
+                current=self.current,
+                current_u=self._current_u,
+            )
+        )
 
     def magnetic_field(self, p: u.m, n: numbers.Integral = 1000) -> u.T:
         r"""
@@ -181,20 +198,27 @@ class FiniteStraightWire(Wire):
         electric current
 
     """
+
     @validate_quantities
     def __init__(self, p1: u.m, p2: u.m, current: u.A):
         self.p1 = p1.value
         self.p2 = p2.value
+        self._p1_u = p1.unit
+        self._p2_u = p2.unit
         if np.all(p1 == p2):
             raise ValueError("p1, p2 should not be the same point.")
         self.current = current.value
+        self._current_u = current.unit
 
     def __repr__(self):
-        return "{name}(p1={p1}, p2={p2}, current={current})".format(
+        return "{name}(p1={p1}{p1_u}, p2={p2}{p2_u}, current={current}{current_u})".format(
             name=self.__class__.__name__,
             p1=self.p1,
             p2=self.p2,
-            current=self.current
+            current=self.current,
+            p1_u=self._p1_u,
+            p2_u=self._p2_u,
+            current_u=self._current_u,
         )
 
     def magnetic_field(self, p) -> u.T:
@@ -261,18 +285,23 @@ class InfiniteStraightWire(Wire):
         electric current
 
     """
+
     @validate_quantities
     def __init__(self, direction, p0: u.m, current: u.A):
-        self.direction = direction/np.linalg.norm(direction)
+        self.direction = direction / np.linalg.norm(direction)
         self.p0 = p0.value
+        self._p0_u = p0.unit
         self.current = current.value
+        self._current_u = current.unit
 
     def __repr__(self):
-        return "{name}(direction={direction}, p0={p0}, current={current})".format(
+        return "{name}(direction={direction}, p0={p0}{p0_u}, current={current}{current_u})".format(
             name=self.__class__.__name__,
             direction=self.direction,
             p0=self.p0,
-            current=self.current
+            current=self.current,
+            p0_u=self._p0_u,
+            current_u=self._current_u,
         )
 
     def magnetic_field(self, p) -> u.T:
@@ -320,16 +349,34 @@ class CircularWire(Wire):
         electric current
 
     """
+
+    def __repr__(self):
+        return (
+            "{name}(normal={normal}, center={center}{center_u}, "
+            "radius={radius}{radius_u}, current={current}{current_u})".format(
+                name=self.__class__.__name__,
+                normal=self.normal,
+                center=self.center,
+                radius=self.radius,
+                current=self.current,
+                center_u=self._center_u,
+                radius_u=self._radius_u,
+                current_u=self._current_u,
+            )
+        )
+
     @validate_quantities
-    def __init__(self, normal, center: u.m, radius: u.m,
-                 current: u.A, n=300):
-        self.normal = normal/np.linalg.norm(normal)
+    def __init__(self, normal, center: u.m, radius: u.m, current: u.A, n=300):
+        self.normal = normal / np.linalg.norm(normal)
         self.center = center.value
+        self._center_u = center.unit
         if radius > 0:
             self.radius = radius.value
+            self._radius_u = radius.unit
         else:
             raise ValueError("Radius should bu larger than 0")
         self.current = current.value
+        self._current_u = current.unit
 
         # parametric equation
         # find other two axises in the disc plane
@@ -362,16 +409,6 @@ def curve(t):
         self.roots_legendre = scipy.special.roots_legendre(n)
         self.n = n
 
-    def __repr__(self):
-        return "{name}(normal={normal}, center={center}, \
-radius={radius}, current={current})".format(
-            name=self.__class__.__name__,
-            normal=self.normal,
-            center=self.center,
-            radius=self.radius,
-            current=self.current
-        )
-
     def magnetic_field(self, p) -> u.T:
         r"""
         Calculate magnetic field generated by this wire at position `p`


Test patch:
diff --git a/plasmapy/formulary/tests/test_magnetostatics.py b/plasmapy/formulary/tests/test_magnetostatics.py
index 5531cdcb..5f71331e 100644
--- a/plasmapy/formulary/tests/test_magnetostatics.py
+++ b/plasmapy/formulary/tests/test_magnetostatics.py
@@ -32,6 +32,11 @@ def test_value2(self):
         assert np.all(np.isclose(B2.value, B2_expected.value))
         assert B2.unit == u.T
 
+    def test_repr(self):
+        "Test __repr__ function"
+        B1 = MagneticDipole(self.moment, self.p0)
+        assert repr(B1) == r"MagneticDipole(moment=[0. 0. 1.]A m2, p0=[0. 0. 0.]m)"
+
 
 class Test_GeneralWire:
     def setup_method(self):
@@ -55,6 +60,16 @@ def test_close_cw(self):
         assert np.all(np.isclose(B_cw.value, B_gw_cw.value))
         assert B_cw.unit == B_gw_cw.unit
 
+    def test_repr(self):
+        "Test __repr__ function"
+        gw_cw = self.cw.to_GeneralWire()
+        # round numbers to avoid calculation accuracy mismatch
+        gw_cw.t1 = -3.1516
+        gw_cw.t2 = +3.1516
+        assert repr(
+            gw_cw
+        ) == r"GeneralWire(parametric_eq=curve, t1=-3.1516, t2=3.1516, current=1.0A)"
+
     def test_close_fw(self):
         "Test if the GeneralWire is close to the FiniteWire it converted from"
         gw_fw = self.fw.to_GeneralWire()
@@ -93,7 +108,7 @@ def test_value1(self):
     def test_repr(self):
         "Test __repr__ function"
         fw = FiniteStraightWire(self.p1, self.p2, self.current)
-        assert repr(fw) == r"FiniteStraightWire(p1=[ 0.  0. -1.], p2=[0. 0. 1.], current=1.0)"
+        assert repr(fw) == r"FiniteStraightWire(p1=[ 0.  0. -1.]m, p2=[0. 0. 1.]m, current=1.0A)"
 
 
 class Test_InfiniteStraightWire:
@@ -114,7 +129,7 @@ def test_repr(self):
         "Test __repr__ function"
         iw = InfiniteStraightWire(self.direction, self.p0, self.current)
         assert repr(iw) == \
-            r"InfiniteStraightWire(direction=[0. 1. 0.], p0=[0. 0. 0.], current=1.0)"
+            r"InfiniteStraightWire(direction=[0. 1. 0.], p0=[0. 0. 0.]m, current=1.0A)"
 
 
 class Test_CircularWire:
@@ -150,4 +165,4 @@ def test_repr(self):
         "Test __repr__ function"
         cw = CircularWire(self.normalz, self.center, self.radius, self.current)
         assert repr(cw) == \
-            r"CircularWire(normal=[0. 0. 1.], center=[0. 0. 0.], radius=1.0, current=1.0)"
+            r"CircularWire(normal=[0. 0. 1.], center=[0. 0. 0.]m, radius=1.0m, current=1.0A)"