Update for next scipp #169
Conversation
tests/nxdata_test.py
Outdated
| @@ -216,7 +216,7 @@ def test_auxiliary_signal_causes_load_as_dataset(h5root): | |||
| snx.create_field(data, 'signal', da.data) | |||
| snx.create_field(data, 'xx', da.coords['xx']) | |||
| data = snx.Group(data, definitions=snx.base_definitions()) | |||
| assert_identical(data[...], sc.Dataset({'signal': da.data, 'xx': da.coords['xx']})) | |||
| assert_identical(data[...], sc.DataGroup(signal=da.data, xx=da.coords['xx'])) | |||
There was a problem hiding this comment.
Honestly not sure what happened here. But in my local build, data[...] returns a datagroup. But not in CI.
There was a problem hiding this comment.
I do not know if NeXus guarantees that aux signals have the same number of dims, so maybe we have to consider moving to DataGroup anyway?
There was a problem hiding this comment.
Seems like this happens anyway which is why I had to fix the test. But I don't know what triggers it. I didn't change the implementation of Group
There was a problem hiding this comment.
It probably means that there is an exception somewhere, so it falls back to loading as a datagroup. This likely indicates your broke something elsewhere. Do not change this test!
There was a problem hiding this comment.
It needs to change because signal and xx have different shapes
There was a problem hiding this comment.
Looking at the standard, I see now that the aux-signals are suppose to have the same shape as the original. That is, the existing test was a bit bad. Therefore:
- Change the shape of the aux signal stored in this test.
- Keep checking for
Dataset.
| ds['face1_center'] = vertices[face1_center.values] | ||
| ds['face1_edge'] = vertices[face1_edge.values] | ||
| ds['face2_center'] = vertices[face2_center.values] | ||
| ds = sc.Dataset( |
There was a problem hiding this comment.
Can you also fix this in v1? It's showing up when using load_nexus.
I know it's deprecated but I still have to use it until we handle the transformation chains.
There was a problem hiding this comment.
Done. Unfortunately, there are no tests for v1. So I don't know for sure that it works.
No description provided.