search

scipp / scipp

Multi-dimensional data arrays with labeled dimensions
https://scipp.github.io/
BSD 3-Clause "New" or "Revised" License
116 stars 18 forks source link

Add `to_quantity` and `from_quantity` #2468

Open jl-wynen opened 2 years ago

jl-wynen commented 2 years ago

Do we want to support these?

var.to(unit=None)
sc.to_unit(var, None)

They currently don't work. to_unit raises an exception but to does nothing because None is used to mean 'do not change'.

I am leaning towards not supporting this because those functions are meant for converting between compatible units. So one should use var.unit = None instead.

However, if we decide to not support it, to needs to handle that case and also raise an exception.

EDIT What about the other way around?

sc.scalar(1, None).to(unit='one')
nvaytet commented 2 years ago

There is also 1 * sc.Unit(None) which fails with

TypeError: __init__(): incompatible constructor arguments. The following argument types are supported:
    1. scipp._scipp.core.Unit(arg0: str)

Invoked with: None

Maybe that should be allowed?

jl-wynen commented 2 years ago

This comes from the constructor of Unit. And I don't think this should be allowed. Because we store the unit as either Unit or None.

SimonHeybrock commented 2 years ago

This comes from the constructor of Unit. And I don't think this should be allowed. Because we store the unit as either Unit or None.

Agree.

I also don't think we should support this in to_unit. Is there an example where this is useful? Maybe it can be fixed in a better manner "upstream" in the code?

jl-wynen commented 2 years ago

I ran into this in a case where code constructed a mask using arithmetic and used da.masks['mask'] = to_unit(x, dtype=bool). Right now, we need to use da.masks['mask'].unit = None in a separate line. It would be nice to combine the unit and dtype 'conversion' in a single to-call. But it is not super relevant.

Like I said, my main problem with the current behaviour is that to silently does nothing when passed unit=None.

SimonHeybrock commented 2 years ago

We should consider dedicated functions to do these kinds of conversions. Basically, everything with a unit is a quantity, so I would suggest sc.to_quantity(var, unit). Not sure about the reverse, could be sc.from_quantity(var)?

SimonHeybrock commented 1 year ago

We could also do it in the spirit of np.asarray:

x = sc.asquantity(y, unit='m')

which would either (1) ensure that y has unit 'm', or (2) if y.unit is None, as the unit. This would allow for expressing contracts or expectations about a function's inputs, for example