Test JSON NeXus with ScippNexus #475
Conversation
Why? What needs updating? I thought ScippNexus is using some |
| @pytest.mark.skip("TODO Stream handling with log not implemented") | ||
| def test_stream_object_as_transformation_results_in_warning(): | ||
| builder = NexusBuilder() | ||
| builder.add_component(Source("source")) | ||
| stream_path = "/entry/streamed_nxlog_transform" | ||
| builder.add_stream(Stream(stream_path)) | ||
| builder.add_dataset_at_path("/entry/source/depends_on", stream_path, {}) | ||
|
|
||
| with pytest.warns(UserWarning): | ||
| loaded_data, _ = load_nexus_json_str(builder.json_string) | ||
|
|
||
| # A 0 distance translation is used in place of the streamed transformation | ||
| default = [0, 0, 0] | ||
| assert np.allclose(loaded_data["source_position"].values, default) | ||
| assert loaded_data["source_position"].unit == sc.Unit("m") |
There was a problem hiding this comment.
Can you explain? You are using NexusBuilder, which you mentioned you planned to remove. Was this just copied from some old test?
There was a problem hiding this comment.
This is an old test. I only moved the file which is why it shows up in the diff.
| "string_size": 20, | ||
| "type": "string", | ||
| "name": "offset", | ||
| "values": "1970-01-01T00:00:00Z" |
There was a problem hiding this comment.
Since this is the default, maybe we should choose another value here?
tests/io/load_nexus_json_test.py
Outdated
| # Using `loaded == expected` could fail if the order of events in the | ||
| # bins is different. | ||
| # Since the order is arbitrary, check that the bins have equal weights instead. |
There was a problem hiding this comment.
The order is not arbitrary: bin and group essentially perform a stable sort, i.e., all events mapping to the same bin are in the same order as they were in the input table.
There was a problem hiding this comment.
Is this guaranteed by the API?
tests/io/load_nexus_json_test.py
Outdated
| # Using `loaded == expected` could fail if the order of events in the | ||
| # bins is different. | ||
| # Since the order is arbitrary, check that the bins have equal weights instead. |
| "string_size": 20, | ||
| "type": "string", | ||
| "name": "start", | ||
| "values": "1970-01-01T00:00:00Z" |
There was a problem hiding this comment.
This is the NeXus default, pick another start to ensure it works correctly.
There was a problem hiding this comment.
It looks like it is not handled.
Is "start" the correct name for the attribute? In the even data, it is called "offset".
There was a problem hiding this comment.
start should be correct: https://manual.nexusformat.org/classes/base_classes/NXlog.html
You are right. |
Fixes #454
I only translated a small number of test cases because the majority of old tests are now covered by ScippNexus. The new tests only check that
JSONGroupworks as a replacement forh5.Group. Please check if I missed some case!The type hints in ScippNexus need to be updated for this. Or we make
JSONGroupinherit fromh5.Group.