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.

Sign up for GitHub

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

Jump to bottom

Implemented Quantity-aware wrappers for assert_allclose and linspace #2402

Closed
wants to merge 1 commit into from
Closed

Implemented Quantity-aware wrappers for assert_allclose and linspace #2402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 41 additions & 0 deletions astropy/units/numpy_wrappers.py
View file
Open in desktop
@@ -0,0 +1,41 @@
import numpy as np

from .quantity import Quantity


def assert_allclose(actual, desired, rtol=1.e-7, atol=0, err_msg='', verbose=True):
"""
Raise an assertion if two objects are not equal up to desired tolerance.

This is a :class:`~astropy.units.Quantity`-aware version of
:func:`numpy.testing.assert_allclose`.
"""
if isinstance(actual, Quantity) and isinstance(desired, Quantity):
np.testing.assert_allclose(actual.value, desired.to(actual.unit).value,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had written a similar utility function for testing and called it assert_quantity but with different semantics.

If you write it like that, then at least you have to describe in the docstring that you convert desired to actual and then e.g. atol applies to those numbers.

There's quite a bit of tests in Astropy already where asserts on quantities are split in two lines ... one testing values and the other testing units ... maybe assert_quantity makes sense in addition and you leave this one as is?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I guess there are two use-cases - one where you check if the values are equivalent, and one where you check where they are exactly equal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I guess users could also expect quantities for atol to work?

I think this shows that it's often not 100% straight-forward how to write a quantity-aware wrapper and we have to look at each one and make sure the docstring explains what it really does.

rtol=rtol, atol=atol, err_msg=err_msg, verbose=verbose)
elif isinstance(actual, Quantity):
raise TypeError("If `actual` is a Quantity, `desired` should also be a Quantity")
elif isinstance(desired, Quantity):
raise TypeError("If `desired` is a Quantity, `actual` should also be a Quantity")
else:
np.testing.assert_allclose(actual, desired,
rtol=rtol, atol=atol, err_msg=err_msg, verbose=verbose)


def linspace(start, stop, num=50, endpoint=True, retstep=False, dtype=None):
"""
Return evenly spaced numbers over a specified interval.

This is a :class:`~astropy.units.Quantity`-aware version of
:func:`numpy.linspace`.
"""
if isinstance(start, Quantity) and isinstance(stop, Quantity):
return np.linspace(start.value, stop.to(start.unit).value, num=num,
endpoint=endpoint, retstep=retstep, dtype=dtype) * start.unit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my numpy, linspace doesn't accept a dtype option!?

In [7]: np.linspace(1, 10, dtype=float)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-7-8979a0b673b3> in <module>()
----> 1 np.linspace(1, 10, dtype=float)

TypeError: linspace() got an unexpected keyword argument 'dtype'

In [8]: np.__version__
Out[8]: '1.8.1'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so maybe arguments that don't care about quantities should be passed via **kwargs to avoid this kind of issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed--it should just work as a transparent replacement for the Numpy ones, keeping in mind that the signatures for some of these functions have changed between Numpy versions :/

elif isinstance(start, Quantity):
raise TypeError("If `start` is a Quantity, `stop` should also be a Quantity")
elif isinstance(stop, Quantity):
raise TypeError("If `stop` is a Quantity, `start` should also be a Quantity")
else:
return np.linspace(start, stop, num=num,
endpoint=endpoint, retstep=retstep, dtype=dtype)
39 changes: 39 additions & 0 deletions astropy/units/tests/test_numpy_wrappers.py
View file
Open in desktop
@@ -0,0 +1,39 @@
from ...tests.helper import pytest
from ... import units as u

from ..numpy_wrappers import assert_allclose, linspace


def test_assert_allclose():

assert_allclose([1,2], [1,2])

assert_allclose([1,2] * u.m, [100,200] * u.cm)

with pytest.raises(AssertionError):
assert_allclose([1,2] * u.m, [90,200] * u.cm)

with pytest.raises(TypeError) as exc:
assert_allclose([1,2] * u.m, [100,200])
assert exc.value.args[0] == "If `actual` is a Quantity, `desired` should also be a Quantity"

with pytest.raises(TypeError) as exc:
assert_allclose([1,2], [100,200] * u.cm)
assert exc.value.args[0] == "If `desired` is a Quantity, `actual` should also be a Quantity"


def test_linspace():

assert_allclose(linspace(1, 10, 10), [1,2,3,4,5,6,7,8,9,10])

assert_allclose(linspace(1 * u.m, 10 * u.m, 10), [1,2,3,4,5,6,7,8,9,10] * u.m)

assert_allclose(linspace(100 * u.cm, 10 * u.m, 10), [1,2,3,4,5,6,7,8,9,10] * u.m)

with pytest.raises(TypeError) as exc:
linspace(1 * u.m, 10, 10)
assert exc.value.args[0] == "If `start` is a Quantity, `stop` should also be a Quantity"

with pytest.raises(TypeError) as exc:
linspace(1, 10 * u.m, 10)
assert exc.value.args[0] == "If `stop` is a Quantity, `start` should also be a Quantity"