Unit conversion

[lucascolley]

do we want unit conversion available via the Quantity API? Or is it sufficient to have a combination of unit conversion in the Unit API and quantity construction (somewhere—#7)?

Discussion of typing this at quantity-dev/quantity-api#1 (comment).

[nstarman]

IMO we should have something, even if it's just a convenience method that does

class Quantity[V, U]:
     def to_unit(self, tounit: U) -> Self[V, U]:
          return tounit.do_quantity_unit_conversion(self)  # or whatever the Unit API method is called.

for 3 reasons.

1. Most (all?) current Q classes have a unit conversion method.
2. Having to_unit may be useful to statically tell if something is a Q. Currently it and unit are the only really unique properties of the Protocol.
3. Users kind of expect it, right?

[neutrinoceros]

Having to_unit may be useful to statically tell if something is a Q. Currently it and unit are the only really unique properties of the Protocol.

I'm not sure how strong of an argument this is. Are we really worried about false positives if the Quantity protocol is restricted to just "has unit and value attributes" ? I don't expect this to be a problem often enough to warrant that we add requirements to the protocol.

Users kind of expect it, right?

indeed. I'm much more convinced by this argument 😄
Confusingly, unyt_array specifically as separate to and to_value methods that both correspond to the idea of to_unit discussed here, the difference being that to returns a copy and to_value performs an in-place conversion. I can never remember which is which, so I'd like to propose that to_unit should have a copy keyword argument, that could work as numpy 2 APIs:

• copy=True: a copy is always returned
• copy=False: no copy is made. An exception is raised if copying cannot be avoided
• copy=None: no copy is made, unless it cannot be avoided. No exception should be raised in any case.
  Implementers should be free to choose if they support copy=True and copy=False (but should raise an exception if they don't), only copy=None should be required.

[nstarman]

@neutrinoceros Are you suggesting .to_unit should be able to do in-place mutation of the value and dtype if copy=False?
Or that to_unit not copy the value and unit when constructing the return value?

return quantity_constructor(maybe_copy(value, copy), maybe_copy(unit, copy))

[andrewgsavage]

@neutrinoceros Are you suggesting .to_unit should be able to do in-place mutation of the value and dtype if copy=False?

That's how I'm reading it, analagous to np.astype with units isntead of dtype.

Assuming we have a conversion method, an inplace operator seems reasonable. pint uses .ito as an inplace .to

I'm not sure on the converion method name, to_unit could be understood to return a Unit. maybe we should follow the array-api deals with astype and have a namespace.asquantity and Quantity.asquantity

[nstarman]

That's how I'm reading it, analagous to np.astype with units instead of dtype.

That's how I'm reading it as well, except that I don't think this is analogous to np.astype, which does not do in-place dtype conversions, only no-copy dtype conversions (if possible and copy=False).

@neutrinoceros I am against putting easily accessible in-place mutations in the common API for the following reasons:

1. Not all types support it: tuple, JAX, tensorflow, PyTorch inside jit contexts, Xarray, Dask.
2. It can change the dtype

q = Quantity[Array[int], Unit](..., unit="lyr")
q.in_place_unit_conversion(unit="pc")  # dtype is now float!

IMO a method .to_unit/asunit/in_unit() (whatever it's called) could take a kwarg inplace: bool or have a numpy-style out= kwarg (or have a dedicated helper method as in pint) but that should be a kwarg/option outside of the common API. See https://data-apis.org/array-api/latest/design_topics/copies_views_and_mutation.html for a discussion on why this is tricky and dangerous and largely left out of the Array API. Implementing libraries are of course free to do whatever they want outside the API.

[neutrinoceros]

@neutrinoceros Are you suggesting .to_unit should be able to do in-place mutation of the value and dtype if copy=False?

Sorry for the late reply, but yes, this is what I was suggesting.
However, in hindsight, I think I was coming at this with a wrong angle and wanted to upstream unyt-specific logic to the common API; I actually fully agree with you that mutating APIs should remain hidden as much as possible. Indeed, your example about changing units on an int array (and implicitly changing dtype in the same move) came up a couple times on unyt's issue tracker and I'm still consistently confused by it when it happens.

[nstarman]

Ping @mhvk for your thoughts on the to_unit (or whatever it's called) method. I outlined some reasoning in #6.

[mhvk]

Personally, I prefer to start with the absolute minimum. Just like the Array API does not have array.astype but only an xp.astype method, I think the only requirement should be that a conversion function exists. I don't think this affects the user experience all that much, at least not when we go for a longer name, i.e., I don't see much advantage of q.to_unit(...) vs qp.to_unit(q, new_unit), especially if we are assuming users need an asquantity function already (where asquantity(q, new_unit) could in principle even do units conversion, if wanted that).

Anyway, as said, I'd prefer to do the truly required stuff first - which arguably means to look better at the units API, and how what kind of converters we expect to be needed.

[nstarman]

Fair enough. We should document somewhere the list of things to return to 😆 .