Conversation
github-actions
Bot
changed the title
|
The workaround for |
| expected.coords[name], | ||
| atol=( | ||
| sc.scalar(1e-15, unit=expected.coords[name].unit) | ||
| # TODO: When scipp supports unit=None in .to(), remove this case. |
There was a problem hiding this comment.
So does this fail when one of the coords has unit=None because internally assert_allclose is calling .to(unit=) on the arguments?
Maybe the call to .to() should be removed and we should just check if the units are the same? (thus requiring manual conversion from the user before calling assert_allclose, but maybe that's ok?)
There was a problem hiding this comment.
I think the fix is rather to make .to() accept unit=None, I have made a PR to Scipp to add support for that.
There was a problem hiding this comment.
my question was more: what was the reason for needing to treat the unit is None case?
There was a problem hiding this comment.
After in-person discussion: calling .to(unit=...) is only called on the atol, not on the variables passed.
So ok for the workaround here.
2 participants
Fixes #696
(https://github.com/scipp/ess/actions/runs/31354093981/job/93350340119)