Give quantify errors context

[TomNicholas]

Fix for #42

• tests added
• update what's new

[keewis]

thanks, @TomNicholas. A test checking that the name of the variable that is supposed to get the invalid unit appears in the error message would be great, but otherwise this looks good to me.

I just noticed that since #41 the "Github Action per Pipeline" works. Did you do anything?

[TomNicholas]

A test checking that the name of the variable that is supposed to get the invalid unit appears in the error message would be great

Done.

I just noticed that since #41 the "Github Action per Pipeline" works. Did you do anything?

Not that I'm aware of!

Should there be an equivalent check for DataArray.pint.quantify? What are we even iterating over there - coordinates?

[keewis]

What are we even iterating over there - coordinates?

coordinates and data:

pint-xarray/pint_xarray/conversion.py

Line 227 in 62c15c5

variables = itertools.chain([(obj.name, obj)], obj.coords.items())

I tried to get only a combined code path for both data and coords.

You will probably have to check against self.da.name to find out if the error was raised for a coordinate or the data.

[keewis]

what's the difference between raises_regex and pytest.raises' match parameter?

[keewis]

I guess we can replace the DataArray call with a _to_temp_dataset / _from_temp_dataset` cycle (after asking whether these could be promoted to semi-public, of course).

Would you have the time to finish this PR, @TomNicholas? If not, I can try pushing to your branch.

[TomNicholas]

Hi @keewis , I'm afraid I don't have time to finish this right now, I can do in a month or two though probably. I remember this stalled because I was seeing some behaviour I didn't understand with my solution, so best not to merge without being confident about how it handles all cases...

[keewis]

no worries. I tried to refactor the code in #56 (joining the DataArray and Dataset code paths makes the code much simpler), so you will probably have to update this PR.

[keewis]

@TomNicholas, would you have time to finish this? It is one of the major issues blocking the release of v0.2

[TomNicholas]

Hi @keewis , that should work now, but I did have to change the raised error type to just a ValueError - trying to raise type(e) gives a weird nonsensical concatenated error message like
UndefinedUnitError: Failed to assign units to variable users is not defined in the unit registry

Raising just a ValueError gives a result that is more like "X was the cause of Y`, with X and Y presented on different lines, which I think is clear enough.

[keewis]

great, thanks, @TomNicholas. I have a few comments, but nothing major.

[keewis]

thanks again, @TomNicholas