Add an internal allclose function #35
Conversation
| ------- | ||
| common_unit : u.Unit | ||
| """ | ||
|
|
There was a problem hiding this comment.
I realize this is mainly just an internal testing function, and so any developer will probably have a better sense of the units/quantities s/he will be dealing with, but there could just be a basic sanity check that dimensionality is the same? Just compare that a.in_base() == b.in_base()?
| if atol is None: | ||
| atol = 1e-8 * common_unit | ||
|
|
||
| if rtol is None: |
There was a problem hiding this comment.
Again, a slight nitpick because people using this function are probably going to be fairly familiar with the syntax for atol and rtol, but if rtol is not None and we use the provided-argument for rtol, we could check unit consistency for the provided rtol? If you pass a unit-less rtol, then you get a somewhat vague unyt error
There was a problem hiding this comment.
Good idea, I tried to handle the cases of unitless rtol and/or atol. I think if a value is passed in for either and it's not necessarily with the same unit, unyt should handle it. Let me know what you think of the changes.
This is born out of an issue using
numpy.allcloseon two unyt arrays, the critical part here:When the arguments are unyt arrays, the expression on the right attempts to add a unyt array (
b) to a unitless float (atol). This function attempts to infer a common unit (loosely defined here by which array has units closest to an order of magnitude of 0, i.e. 10^0 = 1) and multipleatolthrough by that unit.This is also the start of an internal testing module, where we should try and develop our own functions to compare things according to own preferred criteria.