Merge pull request #677 from ess-dmsc/661_byte_strings_not_displayed_…

…correctly

Fixing byte strings not being displayed correctly in field widget value edit

.flake8

@@ -6,7 +6,4 @@ select = B,C,E,F,W,T4,B9
 exclude = ui/,build*,definitions
 per-file-ignores =
     nexus_constructor/geometry/__init__.py:F401
-    tests/test_json_writer.py:F811
-    tests/ui_tests/test_stream_fields_widget.py:F811
-    tests/ui_tests/test_ui_field_attrs.py:F811
     tests/ui_tests/test_ui_fields.py:F811,F401

nexus_constructor/component/component.py

@@ -8,6 +8,7 @@
 from nexus_constructor.component.pixel_shape import PixelShape
 from nexus_constructor.component.transformations_list import TransformationsList
 from nexus_constructor.nexus import nexus_wrapper as nx
+
 from nexus_constructor.nexus.nexus_wrapper import (
     get_nx_class,
     get_name_of_node,

nexus_constructor/field_widget.py

@@ -20,7 +20,10 @@
 from nexus_constructor.common_attrs import CommonAttrs
 from nexus_constructor.field_attrs import FieldAttrsDialog
 from nexus_constructor.invalid_field_names import INVALID_FIELD_NAMES
-from nexus_constructor.nexus.nexus_wrapper import create_temporary_in_memory_file
+from nexus_constructor.nexus.nexus_wrapper import (
+    create_temporary_in_memory_file,
+    to_string,
+)
 from nexus_constructor.stream_fields_widget import StreamFieldsWidget
 from nexus_constructor.ui_utils import validate_line_edit
 from nexus_constructor.validators import (
@@ -269,7 +272,7 @@ def value(self) -> Union[h5py.Dataset, h5py.Group, h5py.SoftLink]:
     @value.setter
     def value(self, value):
         if self.field_type == FieldType.scalar_dataset:
-            self.value_line_edit.setText(str(value))
+            self.value_line_edit.setText(to_string(value))
         elif self.field_type == FieldType.array_dataset:
             self.table_view.model.array = value
         elif self.field_type == FieldType.link:

nexus_constructor/nexus/nexus_wrapper.py

@@ -35,14 +35,19 @@ def get_nx_class(group: h5py.Group) -> Optional[str]:
         return None
 
     nx_class = group.attrs[CommonAttrs.NX_CLASS]
-    return decode_bytes_string(nx_class)
+    return to_string(nx_class)
 
 
-def decode_bytes_string(nexus_string):
-    try:
-        return str(nexus_string, encoding="utf8")
-    except TypeError:
-        return nexus_string
+def to_string(input_to_convert: Any) -> str:
+    """
+    Converts to string, assumes utf-8 encoding for bytes
+    Input can be bytes, str, numpy array
+    :param input_to_convert: Dataset value to convert
+    :return: str
+    """
+    if isinstance(input_to_convert, bytes):
+        return input_to_convert.decode("utf-8")
+    return str(input_to_convert)
 
 
 def create_temporary_in_memory_file() -> h5py.File:

tests/conftest.py

@@ -1,5 +1,7 @@
+import uuid
 from unittest.mock import Mock
 
+import h5py
 import pytest
 from PySide2.QtWidgets import QDialog
 
@@ -18,7 +20,7 @@ def template(qtbot) -> QDialog:
 
 @pytest.fixture(scope="function")
 def nexus_wrapper() -> NexusWrapper:
-    nexus_wrapper = NexusWrapper("test")
+    nexus_wrapper = NexusWrapper(str(uuid.uuid4()))
     yield nexus_wrapper
     nexus_wrapper.nexus_file.close()
 
@@ -51,3 +53,22 @@ def change_mapping_filename(filename):
     change_mapping_filename(None)
 
     return pixel_options
+
+
+class InMemoryFile(object):
+    def __init__(self, filename):
+        self.file_obj = h5py.File(
+            filename, mode="x", driver="core", backing_store=False
+        )
+
+    def __enter__(self):
+        return self.file_obj
+
+    def __exit__(self, type, value, traceback):
+        self.file_obj.close()
+
+
+@pytest.fixture
+def file():
+    with InMemoryFile("test_file") as file:
+        yield file

tests/helpers.py

@@ -1,5 +1,3 @@
-import h5py
-import pytest
 from typing import Any
 from nexus_constructor.component.component import Component
 from nexus_constructor.nexus.nexus_wrapper import NexusWrapper
@@ -15,22 +13,3 @@ def add_component_to_file(
     component_group.create_dataset(field_name, data=field_value)
     component = Component(nexus_wrapper, component_group)
     return component
-
-
-class InMemoryFile(object):
-    def __init__(self, filename):
-        self.file_obj = h5py.File(
-            filename, mode="x", driver="core", backing_store=False
-        )
-
-    def __enter__(self):
-        return self.file_obj
-
-    def __exit__(self, type, value, traceback):
-        self.file_obj.close()
-
-
-@pytest.fixture
-def file():
-    with InMemoryFile("test_file") as file:
-        yield file

tests/test_component_factory.py

@@ -4,7 +4,6 @@
 from nexus_constructor.component.component_shape import ComponentShape
 from nexus_constructor.component.pixel_shape import PixelShape
 from nexus_constructor.component.component_factory import create_component
-from nexus_constructor.nexus import nexus_wrapper as nx
 from nexus_constructor.geometry import (
     OFFGeometryNoNexus,
     NoShapeGeometry,
@@ -36,22 +35,24 @@ def add_dataset(group, name, data, attributes=None):
     return dataset
 
 
-def test_GIVEN_an_NXdisk_chopper_group_WHEN_calling_create_component_THEN_component_has_a_ChopperShape():
-    wrapper = nx.NexusWrapper("file_with_chopper")
-    chopper_group = wrapper.create_nx_group(
-        "chopper", "NXdisk_chopper", wrapper.instrument
+def test_GIVEN_an_NXdisk_chopper_group_WHEN_calling_create_component_THEN_component_has_a_ChopperShape(
+    nexus_wrapper,
+):
+    chopper_group = nexus_wrapper.create_nx_group(
+        "chopper", "NXdisk_chopper", nexus_wrapper.instrument
     )
-    new_component = create_component(wrapper, chopper_group)
+    new_component = create_component(nexus_wrapper, chopper_group)
     assert isinstance(new_component._shape, ChopperShape)
     assert isinstance(
         new_component.shape[0], NoShapeGeometry
     ), "Expect chopper component to return NoShapeGeometry as it has insufficient details to create a mesh of the disk shape"
 
 
-def test_GIVEN_an_NXdisk_chopper_group_WHEN_calling_create_component_THEN_component_returns_OFFGeometry_of_chopper():
-    wrapper = nx.NexusWrapper("file_with_chopper")
-    chopper_group = wrapper.create_nx_group(
-        "chopper", "NXdisk_chopper", wrapper.instrument
+def test_GIVEN_an_NXdisk_chopper_group_WHEN_calling_create_component_THEN_component_returns_OFFGeometry_of_chopper(
+    nexus_wrapper,
+):
+    chopper_group = nexus_wrapper.create_nx_group(
+        "chopper", "NXdisk_chopper", nexus_wrapper.instrument
     )
     add_dataset(
         chopper_group,
@@ -78,59 +79,67 @@ def test_GIVEN_an_NXdisk_chopper_group_WHEN_calling_create_component_THEN_compon
     add_dataset(chopper_group, "slits", 6)
     add_dataset(chopper_group, "slit_height", 130.0, attributes={"units": "mm"})
     add_dataset(chopper_group, "radius", 300.0, attributes={"units": "mm"})
-    new_component = create_component(wrapper, chopper_group)
+    new_component = create_component(nexus_wrapper, chopper_group)
     assert isinstance(new_component._shape, ChopperShape)
     assert isinstance(new_component.shape[0], OFFGeometryNoNexus)
     assert (
         len(new_component.shape[0].vertices) > 8
     ), "Expect chopper geometry with many vertices, not just a placeholder cube with 8 vertices"
 
 
-def test_GIVEN_an_nx_group_with_no_shape_WHEN_calling_create_component_THEN_component_has_a_ComponentShape():
-    wrapper = nx.NexusWrapper("file_with_component_with_no_shape")
-    monitor_group = wrapper.create_nx_group("monitor", "NXmonitor", wrapper.instrument)
-    new_component = create_component(wrapper, monitor_group)
+def test_GIVEN_an_nx_group_with_no_shape_WHEN_calling_create_component_THEN_component_has_a_ComponentShape(
+    nexus_wrapper,
+):
+    monitor_group = nexus_wrapper.create_nx_group(
+        "monitor", "NXmonitor", nexus_wrapper.instrument
+    )
+    new_component = create_component(nexus_wrapper, monitor_group)
     assert isinstance(new_component._shape, ComponentShape)
 
 
-def test_GIVEN_an_nx_group_with_shape_WHEN_calling_create_component_THEN_component_returns_OFFGeometry():
-    wrapper = nx.NexusWrapper("file_with_component_with_shape")
-    monitor_group = wrapper.create_nx_group("monitor", "NXmonitor", wrapper.instrument)
-    new_component = create_component(wrapper, monitor_group)
+def test_GIVEN_an_nx_group_with_shape_WHEN_calling_create_component_THEN_component_returns_OFFGeometry(
+    nexus_wrapper,
+):
+    monitor_group = nexus_wrapper.create_nx_group(
+        "monitor", "NXmonitor", nexus_wrapper.instrument
+    )
+    new_component = create_component(nexus_wrapper, monitor_group)
 
-    shape_group = wrapper.create_nx_group(
+    shape_group = nexus_wrapper.create_nx_group(
         "shape", "NXoff_geometry", new_component.group
     )
-    vertices_dataset = wrapper.set_field_value(
+    vertices_dataset = nexus_wrapper.set_field_value(
         shape_group,
         "vertices",
         np.array([[0, 2, -2], [-1, -1, 1], [1, -1, 1]]),
         dtype=np.float,
     )
-    wrapper.set_field_value(
+    nexus_wrapper.set_field_value(
         shape_group, "winding_order", np.array([0, 1, 2]), dtype=np.int32
     )
-    wrapper.set_field_value(shape_group, "faces", np.array([0]), dtype=np.int32)
-    wrapper.set_attribute_value(vertices_dataset, "units", "m")
+    nexus_wrapper.set_field_value(shape_group, "faces", np.array([0]), dtype=np.int32)
+    nexus_wrapper.set_attribute_value(vertices_dataset, "units", "m")
 
     assert isinstance(new_component.shape[0], OFFGeometryNexus)
 
 
-def test_GIVEN_an_NXdetector_group_with_detector_shape_WHEN_calling_create_component_THEN_component_has_a_ComponentShape():
-    wrapper = nx.NexusWrapper("file_with_detector")
-    detector_group = wrapper.create_nx_group(
-        "detector", "NXdetector", wrapper.instrument
+def test_GIVEN_an_NXdetector_group_with_detector_shape_WHEN_calling_create_component_THEN_component_has_a_ComponentShape(
+    nexus_wrapper,
+):
+    detector_group = nexus_wrapper.create_nx_group(
+        "detector", "NXdetector", nexus_wrapper.instrument
     )
-    wrapper.create_nx_group("detector_shape", "NXoff_geometry", detector_group)
-    new_component = create_component(wrapper, detector_group)
+    nexus_wrapper.create_nx_group("detector_shape", "NXoff_geometry", detector_group)
+    new_component = create_component(nexus_wrapper, detector_group)
     assert isinstance(new_component._shape, ComponentShape)
 
 
-def test_GIVEN_an_NXdetector_group_with_pixel_shape_WHEN_calling_create_component_THEN_component_has_a_PixelShape():
-    wrapper = nx.NexusWrapper("file_with_detector")
-    detector_group = wrapper.create_nx_group(
-        "detector", "NXdetector", wrapper.instrument
+def test_GIVEN_an_NXdetector_group_with_pixel_shape_WHEN_calling_create_component_THEN_component_has_a_PixelShape(
+    nexus_wrapper,
+):
+    detector_group = nexus_wrapper.create_nx_group(
+        "detector", "NXdetector", nexus_wrapper.instrument
     )
-    wrapper.create_nx_group("pixel_shape", "NXoff_geometry", detector_group)
-    new_component = create_component(wrapper, detector_group)
+    nexus_wrapper.create_nx_group("pixel_shape", "NXoff_geometry", detector_group)
+    new_component = create_component(nexus_wrapper, detector_group)
     assert isinstance(new_component._shape, PixelShape)

tests/test_component_fields.py

@@ -1,4 +1,3 @@
-from nexus_constructor.nexus.nexus_wrapper import NexusWrapper
 from nexus_constructor.component.component import Component, add_fields_to_component
 import numpy as np
 
@@ -26,11 +25,12 @@ def __init__(self, name, value, dtype):
         self.dtype = dtype
 
 
-def test_GIVEN_single_scalar_field_and_float_WHEN_adding_fields_to_component_THEN_field_appears_in_component_fields_with_correct_name_and_value():
-    file = NexusWrapper("test_fields_1")
+def test_GIVEN_single_scalar_field_and_float_WHEN_adding_fields_to_component_THEN_field_appears_in_component_fields_with_correct_name_and_value(
+    nexus_wrapper,
+):
 
-    component_group = file.nexus_file.create_group("test_component")
-    component = Component(file, component_group)
+    component_group = nexus_wrapper.nexus_file.create_group("test_component")
+    component = Component(nexus_wrapper, component_group)
 
     field_name = "test_field"
     field_dtype = np.float32
@@ -48,11 +48,12 @@ def test_GIVEN_single_scalar_field_and_float_WHEN_adding_fields_to_component_THE
     assert component.get_field(field_name)[...] == field_value
 
 
-def test_GIVEN_single_scalar_field_and_string_WHEN_adding_fields_to_component_THEN_field_appears_in_component_fields_with_correct_name_and_value():
-    file = NexusWrapper("test_fields_2")
+def test_GIVEN_single_scalar_field_and_string_WHEN_adding_fields_to_component_THEN_field_appears_in_component_fields_with_correct_name_and_value(
+    nexus_wrapper,
+):
 
-    component_group = file.nexus_file.create_group("test_component")
-    component = Component(file, component_group)
+    component_group = nexus_wrapper.nexus_file.create_group("test_component")
+    component = Component(nexus_wrapper, component_group)
 
     field_name = "test_field"
     field_value = np.string_(b"some_value")

tests/test_component_tree_model.py

@@ -499,9 +499,8 @@ def test_duplicate_transform_fail():
     assert False  # Failure
 
 
-def test_remove_component():
-    wrapper = NexusWrapper("test_remove_component")
-    instrument = Instrument(wrapper, NX_CLASS_DEFINITIONS)
+def test_remove_component(nexus_wrapper):
+    instrument = Instrument(nexus_wrapper, NX_CLASS_DEFINITIONS)
     under_test = ComponentTreeModel(instrument)
     instrument.create_component("Some name", "some class", "desc")
     component_index = under_test.index(0, 0, QModelIndex())
@@ -510,9 +509,8 @@ def test_remove_component():
     assert under_test.rowCount(QModelIndex()) == 0
 
 
-def test_remove_transformation():
-    wrapper = NexusWrapper("test_remove_transformation")
-    instrument = Instrument(wrapper, NX_CLASS_DEFINITIONS)
+def test_remove_transformation(nexus_wrapper):
+    instrument = Instrument(nexus_wrapper, NX_CLASS_DEFINITIONS)
     under_test = ComponentTreeModel(instrument)
     instrument.create_component("Some name", "some class", "desc")
     component_index = under_test.index(0, 0, QModelIndex())
@@ -524,9 +522,8 @@ def test_remove_transformation():
     assert under_test.rowCount(transformation_list_index) == 0
 
 
-def test_remove_link():
-    wrapper = NexusWrapper("test_remove_link")
-    instrument = Instrument(wrapper, NX_CLASS_DEFINITIONS)
+def test_remove_link(nexus_wrapper):
+    instrument = Instrument(nexus_wrapper, NX_CLASS_DEFINITIONS)
     under_test = ComponentTreeModel(instrument)
     instrument.create_component("Some name", "some class", "desc")
     component_index = under_test.index(0, 0, QModelIndex())
@@ -539,9 +536,11 @@ def test_remove_link():
     assert under_test.rowCount(transformation_list_index) == 0
 
 
-def test_GIVEN_component_with_cylindrical_shape_information_WHEN_duplicating_component_THEN_shape_information_is_stored_in_nexus_file():
-    wrapper = NexusWrapper("test_duplicate_cyl_shape")
-    instrument = Instrument(wrapper, NX_CLASS_DEFINITIONS)
+def test_GIVEN_component_with_cylindrical_shape_information_WHEN_duplicating_component_THEN_shape_information_is_stored_in_nexus_file(
+    nexus_wrapper,
+):
+
+    instrument = Instrument(nexus_wrapper, NX_CLASS_DEFINITIONS)
 
     first_component_name = "component1"
     first_component_nx_class = "NXdetector"
@@ -571,9 +570,10 @@ def test_GIVEN_component_with_cylindrical_shape_information_WHEN_duplicating_com
     assert second_shape.units == units
 
 
-def test_GIVEN_component_with_off_shape_information_WHEN_duplicating_component_THEN_shape_information_is_stored_in_nexus_file():
-    wrapper = NexusWrapper("test_duplicate_off_shape")
-    instrument = Instrument(wrapper, NX_CLASS_DEFINITIONS)
+def test_GIVEN_component_with_off_shape_information_WHEN_duplicating_component_THEN_shape_information_is_stored_in_nexus_file(
+    nexus_wrapper,
+):
+    instrument = Instrument(nexus_wrapper, NX_CLASS_DEFINITIONS)
 
     first_component_name = "component1"
     first_component_nx_class = "NXdetector"