From b7a2bb758db6243e12536d7f7c67a5f2c384dae4 Mon Sep 17 00:00:00 2001 From: "Sode, Adedamola (DLSLtd,RAL,LSCI)" Date: Tue, 5 May 2026 10:22:51 +0000 Subject: [PATCH 1/7] Added label feature for the techui.yaml reverting an unnecessary change modified generate_tests for coverage generate_test coverage Removed unneeded prints and logs --- example/t01-services/synoptic/techui.yaml | 10 ++- src/techui_builder/autofill.py | 6 +- src/techui_builder/builder.py | 93 ++++++++++++++++++++--- src/techui_builder/generate.py | 31 +++++++- src/techui_builder/models.py | 9 ++- tests/conftest.py | 4 +- tests/test_autofiller.py | 2 +- tests/test_builder.py | 68 ++++++++++++++--- tests/test_files/widget.xml | 1 + tests/test_generate.py | 56 +++++++++++++- tests/test_models.py | 15 ++-- 11 files changed, 256 insertions(+), 39 deletions(-) diff --git a/example/t01-services/synoptic/techui.yaml b/example/t01-services/synoptic/techui.yaml index be06909..341bfaf 100644 --- a/example/t01-services/synoptic/techui.yaml +++ b/example/t01-services/synoptic/techui.yaml @@ -7,16 +7,20 @@ beamline: components: fshtr: - desc: Fast Shutter + label: Fast Shutter prefix: BL01T-EA-FSHTR-01 d1: - desc: Diode 1 + label: Diode 1 prefix: BL01T-DI-PHDGN-01 file: test.bob motor: - desc: Motor Stage + label: Motor Stage prefix: BL01T-MO-MOTOR-01 extras: - BL01T-MO-BRICK-01 + child_labels: + X: X1 + Y: Y1 + Z: Z1 diff --git a/src/techui_builder/autofill.py b/src/techui_builder/autofill.py index 63b7e68..0443342 100644 --- a/src/techui_builder/autofill.py +++ b/src/techui_builder/autofill.py @@ -91,7 +91,11 @@ def replace_content( tag_name = "description" if component_attr is None: - component_attr = component_name + component_attr = ( + component_name + if component.label is None + else component.label + ) case "file": tag_name = "file" diff --git a/src/techui_builder/builder.py b/src/techui_builder/builder.py index 8144b2f..69633e6 100644 --- a/src/techui_builder/builder.py +++ b/src/techui_builder/builder.py @@ -13,7 +13,7 @@ from softioc.builder import records from techui_builder.generate import Generator -from techui_builder.models import Entity, TechUi +from techui_builder.models import Component, Entity, TechUi from techui_builder.validator import Validator logger_ = logging.getLogger(__name__) @@ -66,7 +66,9 @@ def setup(self): self.clean_files() - self.generator = Generator(synoptic_dir, self.conf.beamline.url) + self.generator = Generator( + synoptic_dir, self.conf.beamline.url, self.conf.components + ) def clean_files(self): exclude = {"index.bob"} @@ -220,7 +222,13 @@ def create_screens(self): # ONLY IF there is a matching component and entity, generate a screen if component.prefix in self.entities.keys(): + # Populate child labels for any entities + # with the same prefix as the component + for entity in self.entities[component.prefix]: + entity.child_labels = component.child_labels + screen_entities.extend(self.entities[component.prefix]) + if component.extras is not None: # If component has any extras, add them to the entries to generate for extra_p in component.extras: @@ -251,7 +259,14 @@ def create_screens(self): " any P field in the ioc.yaml files in services" ) - def _generate_json_map(self, screen_path: Path, dest_path: Path) -> JsonMap: + def _generate_json_map( + self, + screen_path: Path, + dest_path: Path, + component: dict[str, Component], + current_component_name: str | None = None, + name_elem: str | None = None, + ) -> JsonMap: """Recursively generate JSON map from .bob file tree""" # Create initial node at top of .bob file @@ -260,6 +275,10 @@ def _generate_json_map(self, screen_path: Path, dest_path: Path) -> JsonMap: display_name=None, ) + # Get Current Component + if current_component_name is None and screen_path.stem in component: + current_component_name = screen_path.stem + abs_path = screen_path.absolute() try: @@ -271,7 +290,12 @@ def _generate_json_map(self, screen_path: Path, dest_path: Path) -> JsonMap: current_node.display_name = self._parse_display_name( root.name.text, screen_path ) - + current_node.display_name = _get_labels( + name_elem, + component, + current_component_name, + current_node.display_name, + ) # Find all elements widgets = [ w @@ -307,6 +331,15 @@ def _generate_json_map(self, screen_path: Path, dest_path: Path) -> JsonMap: case _: continue + # Validated screen names don't get renegerated + display_name = name_elem + display_name = _get_labels( + name_elem, + component, + current_component_name, + display_name, + ) + # Extract file path from file_elem file_path = Path(file_elem.text.strip() if file_elem.text else "") @@ -315,7 +348,7 @@ def _generate_json_map(self, screen_path: Path, dest_path: Path) -> JsonMap: continue # Create valid displayName - display_name = self._parse_display_name(name_elem, file_path) + display_name = self._parse_display_name(display_name, file_path) # TODO: misleading var name? next_file_path = dest_path.joinpath(file_path) @@ -323,7 +356,13 @@ def _generate_json_map(self, screen_path: Path, dest_path: Path) -> JsonMap: # Crawl the next file if next_file_path.is_file(): # TODO: investigate non-recursive approaches? - child_node = self._generate_json_map(next_file_path, dest_path) + child_node = self._generate_json_map( + next_file_path, + dest_path, + component, + current_component_name=current_component_name, + name_elem=name_elem, + ) else: child_node = JsonMap( str(file_path), display_name, exists=("IOC" in macro_dict) @@ -340,7 +379,7 @@ def _generate_json_map(self, screen_path: Path, dest_path: Path) -> JsonMap: except Exception as e: current_node.error = str(e) - self._fix_duplicate_names(current_node) + self._fix_names_json_map(current_node, component) return current_node @@ -398,7 +437,9 @@ def _parse_display_name(self, name: str | None, file_path: Path) -> str | None: # Populate displayName with null return None - def _fix_duplicate_names(self, node: JsonMap) -> None: + def _fix_names_json_map( + self, node: JsonMap, components: dict[str, Component] + ) -> None: """Recursively fix duplicate display names in children""" if not node.children: return @@ -412,6 +453,7 @@ def _fix_duplicate_names(self, node: JsonMap) -> None: for name, children in name_groups.items(): if name and len(children) > 1: # append pv names when present + for child in children: if "P" in child.macros: child.display_name = f"{name} ({child.macros['P']})" @@ -423,7 +465,7 @@ def _fix_duplicate_names(self, node: JsonMap) -> None: # recursively fix children for child in node.children: - self._fix_duplicate_names(child) + self._fix_names_json_map(child, components) def write_json_map( self, @@ -439,7 +481,7 @@ def write_json_map( f"Cannot generate json map for {synoptic}. Has it been generated?" ) - map = self._generate_json_map(synoptic, dest) + map = self._generate_json_map(synoptic, dest, self.conf.components) with open(dest.joinpath("JsonMap.json"), "w") as f: f.write( json.dumps(map, indent=4, default=lambda o: _serialise_json_map(o)) @@ -504,3 +546,34 @@ def _get_action_group(element: ObjectifiedElement) -> ObjectifiedElement | None: f"Actions group not found in component [bold]{name}[/bold] on " f"[bold]{parent_name}[/bold]" ) + + +def _get_labels( + name_elem: str | None, + component: dict[str, Component], + current_component_name: str | None, + display_name: str | None, +) -> str | None: + """ + Get display name from child labels if they exist, otherwise return name_elem + or existing display_name if name_elem is None. + """ + if name_elem is not None: + if name_elem in component.keys() and component[name_elem].label is not None: + display_name = component[name_elem].label + elif current_component_name is not None and ( + component[current_component_name].child_labels is not None + ): + child_labels = component[current_component_name].child_labels + if child_labels is not None: + # Because name_elem is initially + # grabbed from the .bob file, the generated .bob + # file might have already propagated the child label from techui.yaml + if name_elem in child_labels.values(): + display_name = name_elem + # In the case of screens not regenerated, such as validated screens, + # the name text will not be updated to the childlabel,so we check the + # keys solely for generating the json_map from the top level .bob file + elif name_elem in child_labels: + display_name = child_labels[name_elem] + return display_name diff --git a/src/techui_builder/generate.py b/src/techui_builder/generate.py index 4fba461..7497f51 100644 --- a/src/techui_builder/generate.py +++ b/src/techui_builder/generate.py @@ -12,7 +12,7 @@ from phoebusgen import widget as pwidget from phoebusgen.widget.widgets import ActionButton, EmbeddedDisplay, Group -from techui_builder.models import Entity +from techui_builder.models import Component, Entity logger_ = logging.getLogger(__name__) @@ -21,6 +21,7 @@ class Generator: synoptic_dir: Path = field(repr=False) beamline_url: str = field(repr=False) + components: dict[str, Component] = field(default_factory=dict, repr=False) # These are global params for the class (not accessible by user) support_path: Path = field(init=False, repr=False) @@ -40,6 +41,7 @@ class Generator: widget_x: int = field(default=0, init=False, repr=False) widget_count: int = field(default=0, init=False, repr=False) group_padding: int = field(default=50, init=False, repr=False) + label_flag: bool = field(default=False, init=False, repr=False) def __post_init__(self): # This needs to be before _read_map() @@ -173,6 +175,18 @@ def _initialise_name_suffix(self, component: Entity) -> tuple[str, str, str | No suffix = "" suffix_label = "" + # Try to get name from child labels if they exist, + # if not, just use the name as it is. + if component.child_labels is not None: + if ( + name.removeprefix(":").removesuffix(":") + in component.child_labels.keys() + ): + name = component.child_labels[name.removeprefix(":").removesuffix(":")] + self.label_flag = True + + logger_.debug(f"Name after child label check: {name}") + return (name, suffix, suffix_label) def _is_list_of_dicts(self, scrn_mapping: Mapping) -> bool: @@ -201,7 +215,8 @@ def _allocate_widget( ) if match: suffix_label: str | None = match.group(2) - name: str = suffix + if self.label_flag is False: + name = suffix except KeyError: pass @@ -221,6 +236,7 @@ def _allocate_widget( new_widget.macro( f"{suffix_label}", suffix.removeprefix(":").removesuffix(":") ) + new_widget.macro("label", name.removeprefix(":").removesuffix(":")) # TODO: Change this to pvi_button if True: new_widget.macro("IOC", f"{self.beamline_url}/{component.service_name}") @@ -260,6 +276,7 @@ def _allocate_widget( # For some reason the version of action buttons is 3.0.0? new_widget.version("2.0.0") + self.label_flag = False return new_widget def _create_widget( @@ -381,8 +398,16 @@ def build_groups(self, screen_name: str): # that will be created. height, width = self._get_group_dimensions(self.widgets) + if ( + screen_name in self.components + and self.components[screen_name].label is not None + ): + label = self.components[screen_name].label or screen_name + else: + label = screen_name + self.group = Group( - screen_name, + label, 0, 0, width, diff --git a/src/techui_builder/models.py b/src/techui_builder/models.py index bb26d69..61e6440 100644 --- a/src/techui_builder/models.py +++ b/src/techui_builder/models.py @@ -106,7 +106,10 @@ class Component(BaseModel): """One UI Component from techui.yaml `components:` dictionary""" prefix: Annotated[str, Field(description="Component PV Prefix")] - desc: Annotated[str | None, Field(description="Component description")] = None + label: Annotated[str | None, Field(description="Component label")] = None + child_labels: Annotated[ + dict[str, str] | None, Field(description="Component Children Label") + ] = None extras: Annotated[ list[str] | None, Field( @@ -273,6 +276,10 @@ class Entity(BaseModel): desc: Annotated[ str | None, Field(description="Optional description of module entity") ] = None + child_labels: Annotated[ + dict[str, str] | None, + Field(description="Optional child labels for module entity"), + ] = None M: Annotated[str | None, Field(description="Optional PV suffix for a motor")] R: Annotated[ str | None, Field(description="Optional PV suffix for an ADAravis plugin") diff --git a/tests/conftest.py b/tests/conftest.py index 1c05c40..9ba0a69 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -109,10 +109,10 @@ def example_display_names_json(): @pytest.fixture -def generator(): +def generator(builder: Builder): synoptic_dir = Path(__file__).parent.joinpath(Path("t01-services/synoptic")) - g = Generator(synoptic_dir, "test_url") + g = Generator(synoptic_dir, "test_url", builder.conf.components) return g diff --git a/tests/test_autofiller.py b/tests/test_autofiller.py index aeae4bc..81af83f 100644 --- a/tests/test_autofiller.py +++ b/tests/test_autofiller.py @@ -87,7 +87,7 @@ def test_autofiller_replace_content( # Cannot use a Mock object as need P to be computed fake_component = Component( prefix=prefix, - desc=description, + label=description, file=filename, macros=macros, ) diff --git a/tests/test_builder.py b/tests/test_builder.py index c1ce6ac..358a930 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -12,6 +12,7 @@ from techui_builder.builder import ( JsonMap, _get_action_group, # type: ignore + _get_labels, # type: ignore _serialise_json_map, # type: ignore ) @@ -29,10 +30,10 @@ def test_beamline_attributes(builder, attr, expected): @pytest.mark.parametrize( - "index, name, desc, P, R, attribute, file, extras", + "index, name, label, P, R, attribute, file, extras, child_labels", [ - (0, "fshtr", "Fast Shutter", "BL01T-EA-FSHTR-01", None, None, None, None), - (1, "d1", "Diode 1", "BL01T-DI-PHDGN-01", None, None, "test.bob", None), + (0, "fshtr", "Fast Shutter", "BL01T-EA-FSHTR-01", None, None, None, None, None), + (1, "d1", "Diode 1", "BL01T-DI-PHDGN-01", None, None, "test.bob", None, None), ( 2, "motor", @@ -42,6 +43,7 @@ def test_beamline_attributes(builder, attr, expected): None, None, None, + {"X": "X1", "Y": "Y1", "Z": "Z1"}, ), ], ) @@ -49,20 +51,22 @@ def test_component_attributes( builder, index, name, - desc, + label, P, # noqa: N803 R, # noqa: N803 attribute, file, extras, + child_labels, ): components = list(builder.conf.components.keys()) component = builder.conf.components[components[index]] assert components[index] == name - assert component.desc == desc + assert component.label == label assert component.P == P assert component.R == R assert component.attribute == attribute + assert component.child_labels == child_labels if file is not None: assert component.file == file if extras is not None: @@ -301,7 +305,7 @@ def test_generate_json_map( mock_get_action_group.return_value = mock_xml test_json_map = builder_with_test_files._generate_json_map( - screen_path.absolute(), dest_path + screen_path.absolute(), dest_path, builder_with_test_files.conf.components ) assert test_json_map == example_json_map @@ -312,7 +316,9 @@ def test_generate_json_map_embedded_screen(builder_with_test_files, example_json screen_path = Path("tests/test_files/test_bob_embedded.bob").absolute() dest_path = Path("tests/test_files/") - test_json_map = builder_with_test_files._generate_json_map(screen_path, dest_path) + test_json_map = builder_with_test_files._generate_json_map( + screen_path, dest_path, builder_with_test_files.conf.components + ) example_json_map.file = "test_bob_embedded.bob" example_json_map.children.append( JsonMap( @@ -343,7 +349,7 @@ def test_parse_display_name_returns_none(builder): assert display_name is None -def test_fix_duplicate_names_recursive(builder, example_display_names_json): +def test_fix_names_json_map_recursive(builder, example_display_names_json): """Test duplicate names are enumerated correctly for all children""" test_display_names_json = JsonMap( @@ -377,7 +383,7 @@ def test_fix_duplicate_names_recursive(builder, example_display_names_json): test_display_names_json.children.append(test_display_names_json_dev1) test_display_names_json.children.append(test_display_names_json_dev2) - builder._fix_duplicate_names(test_display_names_json) + builder._fix_names_json_map(test_display_names_json, builder.conf.components) assert test_display_names_json == example_display_names_json @@ -399,7 +405,9 @@ def test_generate_json_map_get_macros( macros["macro"] = "value" mock_get_action_group.return_value = mock_xml - test_json_map = builder_with_test_files._generate_json_map(screen_path, dest_path) + test_json_map = builder_with_test_files._generate_json_map( + screen_path, dest_path, builder_with_test_files.conf.components + ) assert test_json_map == example_json_map @@ -408,7 +416,9 @@ def test_generate_json_map_xml_parse_error(builder_with_test_files, test_files): screen_path = Path("tests/test_files/test_bob_bad.bob").absolute() _, dest_path = test_files - test_json_map = builder_with_test_files._generate_json_map(screen_path, dest_path) + test_json_map = builder_with_test_files._generate_json_map( + screen_path, dest_path, builder_with_test_files.conf.components + ) assert test_json_map.error.startswith("XML parse error:") @@ -421,7 +431,9 @@ def test_generate_json_map_other_exception( mock_get_action_group.side_effect = Exception("Some exception") - test_json_map = builder_with_test_files._generate_json_map(screen_path, dest_path) + test_json_map = builder_with_test_files._generate_json_map( + screen_path, dest_path, builder_with_test_files.conf.components + ) assert test_json_map.error != "" @@ -472,3 +484,35 @@ def test_get_action_group_no_actions_group(caplog): for log_output in caplog.records: assert "Actions group not found" in log_output.message + + +def test_get_labels(builder_with_test_files): + display_name = _get_labels( + "motor", + builder_with_test_files.conf.components, + None, + None, + ) + assert display_name == "Motor Stage" + + +def test_get_labels_child_labels(builder_with_test_files): + display_name = _get_labels( + "X", + builder_with_test_files.conf.components, + current_component_name="motor", + display_name="X", + ) + assert display_name == "X1" + + +def test_get_labels_child_labels_with_name_already_pregenerated( + builder_with_test_files, +): + display_name = _get_labels( + "X1", + builder_with_test_files.conf.components, + current_component_name="motor", + display_name="X", + ) + assert display_name == "X1" diff --git a/tests/test_files/widget.xml b/tests/test_files/widget.xml index 10ddc40..63cb651 100644 --- a/tests/test_files/widget.xml +++ b/tests/test_files/widget.xml @@ -9,6 +9,7 @@

BL01T-DI-IOC-01

CAM + test_url/bl01t-di-ioc-01
diff --git a/tests/test_generate.py b/tests/test_generate.py index 7fbc9ad..6c3ec63 100644 --- a/tests/test_generate.py +++ b/tests/test_generate.py @@ -212,6 +212,24 @@ def test_generator_initialise_name_suffix_none(generator): assert suffix_label == "" +def test_generator_initialise_name_suffix_with_child_labels(generator): + component = Entity( + type="test", + P="TEST", + desc=None, + service_name="bl01t-mo-test-01", + M=None, + R="T1", + child_labels={"T1": "Test 1"}, + ) + + name, suffix, suffix_label = generator._initialise_name_suffix(component) + + assert name == "Test 1" + assert suffix == "T1" + assert suffix_label == "R" + + def test_generator_is_list_of_dicts(generator): list_of_dicts = [{"a": 1}, {"b": 2}] assert generator._is_list_of_dicts(list_of_dicts) is True @@ -248,7 +266,7 @@ def test_generator_allocate_widget(generator): def test_generator_allocate_widget_with_suffix(generator): - generator._initilise_name_suffix = Mock(return_value=(":CAM:", ":CAM:", "R")) + generator._initialise_name_suffix = Mock(return_value=(":CAM:", ":CAM:", "R")) scrn_mapping = { "file": "ADAravis/ADAravis_summary.bob", @@ -379,6 +397,42 @@ def test_generator_build_screen(generator): assert objectify.fromstring(str(generator.screen_)).xpath("//widget[@type='group']") +def test_build_groups_with_label(generator): + screen_name = "motor" + generator.widgets = [Mock(), Mock(), Mock()] + generator._create_widget = Mock(return_value=Mock()) + generator.layout_widgets = Mock( + return_value=[ + pwidget.EmbeddedDisplay(name="X", file="", x=0, y=0, width=205, height=120), + pwidget.EmbeddedDisplay( + name="Y", file="", x=0, y=150, width=205, height=120 + ), + ] + ) + generator._get_group_dimensions = Mock(return_value=(600, 400)) + generator.build_groups(screen_name) + xml = objectify.fromstring(str(generator.group)) + assert xml.xpath("//name")[0] == "Motor Stage" + + +def test_build_groups(generator): + screen_name = "test" + generator.widgets = [Mock(), Mock(), Mock()] + generator._create_widget = Mock(return_value=Mock()) + generator.layout_widgets = Mock( + return_value=[ + pwidget.EmbeddedDisplay(name="X", file="", x=0, y=0, width=205, height=120), + pwidget.EmbeddedDisplay( + name="Y", file="", x=0, y=150, width=205, height=120 + ), + ] + ) + generator._get_group_dimensions = Mock(return_value=(600, 400)) + generator.build_groups(screen_name) + xml = objectify.fromstring(str(generator.group)) + assert xml.xpath("//name")[0] == "test" + + def test_generator_write_screen(generator): screen_name = "test" generator.screen_ = pscreen.Screen("test") diff --git a/tests/test_models.py b/tests/test_models.py index e6a8cd5..a31d73f 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -21,7 +21,10 @@ def beamline() -> Beamline: @pytest.fixture def component() -> Component: return Component( - prefix="BL01T-EA-TEST-02", desc="Test Device", status=["BL01T-MO-MOTOR-01:Y"] + prefix="BL01T-EA-TEST-02", + label="Test Device", + status=["BL01T-MO-MOTOR-01:Y"], + child_labels={"X": "X1", "Y": "Y1", "Z": "Z1"}, ) @@ -41,25 +44,27 @@ def test_beamline_object(beamline: Beamline): def test_component_object(component: Component): - assert component.desc == "Test Device" + assert component.label == "Test Device" assert component.extras is None assert component.P == "BL01T-EA-TEST-02" assert component.R is None assert component.attribute is None assert component.status == ["BL01T-MO-MOTOR-01:Y"] + assert component.child_labels == {"X": "X1", "Y": "Y1", "Z": "Z1"} def test_component_repr(component: Component): assert ( str(component) - == "prefix='BL01T-EA-TEST-02' desc='Test Device' extras=None\ - file=None macros=None status=['BL01T-MO-MOTOR-01:Y']" + == "prefix='BL01T-EA-TEST-02' label='Test Device' child_labels\ +={'X': 'X1', 'Y': 'Y1', 'Z': 'Z1'} extras=None file=None macros=None\ + status=['BL01T-MO-MOTOR-01:Y']" ) def test_component_bad_prefix(): with pytest.raises(ValueError): - Component(prefix="Test 2", desc="BAD_PREFIX") + Component(prefix="Test 2", label="BAD_PREFIX") def test_gui_component_entry(gui_components: GuiComponentEntry): From a22f32799799b850a42468cecdabe252337a15cc Mon Sep 17 00:00:00 2001 From: "Sode, Adedamola (DLSLtd,RAL,LSCI)" Date: Thu, 7 May 2026 07:04:10 +0000 Subject: [PATCH 2/7] Made recommended changes --- src/techui_builder/builder.py | 12 +++--- src/techui_builder/generate.py | 23 +++++------- tests/conftest.py | 9 ++++- tests/test_builder.py | 68 ++++++++++++++++++++++++++-------- tests/test_generate.py | 14 ++++--- 5 files changed, 82 insertions(+), 44 deletions(-) diff --git a/src/techui_builder/builder.py b/src/techui_builder/builder.py index 69633e6..418e3bb 100644 --- a/src/techui_builder/builder.py +++ b/src/techui_builder/builder.py @@ -66,9 +66,7 @@ def setup(self): self.clean_files() - self.generator = Generator( - synoptic_dir, self.conf.beamline.url, self.conf.components - ) + self.generator = Generator(synoptic_dir, self.conf.beamline.url) def clean_files(self): exclude = {"index.bob"} @@ -243,7 +241,7 @@ def create_screens(self): # This is used by both generate and validate, # so called beforehand for tidyness self.generator.build_widgets(component_name, screen_entities) - self.generator.build_groups(component_name) + self.generator.build_groups(component_name, self.conf.components) screens_to_validate = list(self.validator.validate.keys()) @@ -561,8 +559,10 @@ def _get_labels( if name_elem is not None: if name_elem in component.keys() and component[name_elem].label is not None: display_name = component[name_elem].label - elif current_component_name is not None and ( - component[current_component_name].child_labels is not None + elif ( + current_component_name is not None + and (current_component_name in component.keys()) + and (component[current_component_name].child_labels is not None) ): child_labels = component[current_component_name].child_labels if child_labels is not None: diff --git a/src/techui_builder/generate.py b/src/techui_builder/generate.py index 7497f51..cb3625a 100644 --- a/src/techui_builder/generate.py +++ b/src/techui_builder/generate.py @@ -21,7 +21,6 @@ class Generator: synoptic_dir: Path = field(repr=False) beamline_url: str = field(repr=False) - components: dict[str, Component] = field(default_factory=dict, repr=False) # These are global params for the class (not accessible by user) support_path: Path = field(init=False, repr=False) @@ -175,18 +174,14 @@ def _initialise_name_suffix(self, component: Entity) -> tuple[str, str, str | No suffix = "" suffix_label = "" - # Try to get name from child labels if they exist, - # if not, just use the name as it is. + name = name.removeprefix(":").removesuffix(":") + # Try to get name from child labels if they exist, + # if not, just use the name as it is. if component.child_labels is not None: - if ( - name.removeprefix(":").removesuffix(":") - in component.child_labels.keys() - ): - name = component.child_labels[name.removeprefix(":").removesuffix(":")] + if name in component.child_labels.keys(): + name = component.child_labels[name] self.label_flag = True - logger_.debug(f"Name after child label check: {name}") - return (name, suffix, suffix_label) def _is_list_of_dicts(self, scrn_mapping: Mapping) -> bool: @@ -384,7 +379,7 @@ def build_widgets(self, screen_name: str, screen_components: list[Entity]): continue self.widgets.append(new_widget) - def build_groups(self, screen_name: str): + def build_groups(self, screen_name: str, builder_components: dict[str, Component]): """ Create a group to fill with widgets """ @@ -399,10 +394,10 @@ def build_groups(self, screen_name: str): height, width = self._get_group_dimensions(self.widgets) if ( - screen_name in self.components - and self.components[screen_name].label is not None + screen_name in builder_components.keys() + and builder_components[screen_name].label is not None ): - label = self.components[screen_name].label or screen_name + label = builder_components[screen_name].label or screen_name else: label = screen_name diff --git a/tests/conftest.py b/tests/conftest.py index 9ba0a69..60934c7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -39,6 +39,11 @@ def builder_with_test_files(builder: Builder): return builder +@pytest.fixture +def components(builder_with_test_files: Builder): + return builder_with_test_files.conf.components + + @pytest.fixture def test_files(): screen_path = Path("tests/test_files/test_bob.bob").absolute() @@ -109,10 +114,10 @@ def example_display_names_json(): @pytest.fixture -def generator(builder: Builder): +def generator(): synoptic_dir = Path(__file__).parent.joinpath(Path("t01-services/synoptic")) - g = Generator(synoptic_dir, "test_url", builder.conf.components) + g = Generator(synoptic_dir, "test_url") return g diff --git a/tests/test_builder.py b/tests/test_builder.py index 358a930..86a5448 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -296,7 +296,11 @@ def test_write_json_map(builder): # We don't want to access the _get_action_group function in this test @patch("techui_builder.builder._get_action_group") def test_generate_json_map( - mock_get_action_group, builder_with_test_files, example_json_map, test_files + mock_get_action_group, + builder_with_test_files, + example_json_map, + test_files, + components, ): screen_path, dest_path = test_files @@ -305,19 +309,21 @@ def test_generate_json_map( mock_get_action_group.return_value = mock_xml test_json_map = builder_with_test_files._generate_json_map( - screen_path.absolute(), dest_path, builder_with_test_files.conf.components + screen_path.absolute(), dest_path, components ) assert test_json_map == example_json_map # TODO: write this test -def test_generate_json_map_embedded_screen(builder_with_test_files, example_json_map): +def test_generate_json_map_embedded_screen( + builder_with_test_files, example_json_map, components +): screen_path = Path("tests/test_files/test_bob_embedded.bob").absolute() dest_path = Path("tests/test_files/") test_json_map = builder_with_test_files._generate_json_map( - screen_path, dest_path, builder_with_test_files.conf.components + screen_path, dest_path, components ) example_json_map.file = "test_bob_embedded.bob" example_json_map.children.append( @@ -391,7 +397,11 @@ def test_fix_names_json_map_recursive(builder, example_display_names_json): # We don't want to access the _get_action_group function in this test @patch("techui_builder.builder._get_action_group") def test_generate_json_map_get_macros( - mock_get_action_group, builder_with_test_files, example_json_map, test_files + mock_get_action_group, + builder_with_test_files, + example_json_map, + test_files, + components, ): screen_path, dest_path = test_files @@ -406,18 +416,20 @@ def test_generate_json_map_get_macros( mock_get_action_group.return_value = mock_xml test_json_map = builder_with_test_files._generate_json_map( - screen_path, dest_path, builder_with_test_files.conf.components + screen_path, dest_path, components ) assert test_json_map == example_json_map -def test_generate_json_map_xml_parse_error(builder_with_test_files, test_files): +def test_generate_json_map_xml_parse_error( + builder_with_test_files, test_files, components +): screen_path = Path("tests/test_files/test_bob_bad.bob").absolute() _, dest_path = test_files test_json_map = builder_with_test_files._generate_json_map( - screen_path, dest_path, builder_with_test_files.conf.components + screen_path, dest_path, components ) assert test_json_map.error.startswith("XML parse error:") @@ -425,14 +437,14 @@ def test_generate_json_map_xml_parse_error(builder_with_test_files, test_files): @patch("techui_builder.builder._get_action_group") def test_generate_json_map_other_exception( - mock_get_action_group, builder_with_test_files, test_files + mock_get_action_group, builder_with_test_files, test_files, components ): screen_path, dest_path = test_files mock_get_action_group.side_effect = Exception("Some exception") test_json_map = builder_with_test_files._generate_json_map( - screen_path, dest_path, builder_with_test_files.conf.components + screen_path, dest_path, components ) assert test_json_map.error != "" @@ -486,20 +498,20 @@ def test_get_action_group_no_actions_group(caplog): assert "Actions group not found" in log_output.message -def test_get_labels(builder_with_test_files): +def test_get_labels(components): display_name = _get_labels( "motor", - builder_with_test_files.conf.components, + components, None, None, ) assert display_name == "Motor Stage" -def test_get_labels_child_labels(builder_with_test_files): +def test_get_labels_child_labels(components): display_name = _get_labels( "X", - builder_with_test_files.conf.components, + components, current_component_name="motor", display_name="X", ) @@ -507,12 +519,36 @@ def test_get_labels_child_labels(builder_with_test_files): def test_get_labels_child_labels_with_name_already_pregenerated( - builder_with_test_files, + components, ): display_name = _get_labels( "X1", - builder_with_test_files.conf.components, + components, current_component_name="motor", display_name="X", ) assert display_name == "X1" + + +def test_get_labels_with_name_elem_invalid( + components, +): + display_name = _get_labels( + "invalid_name", + components, + current_component_name=None, + display_name="new_name", + ) + assert display_name == "new_name" + + +def test_get_labels_with_current_component_name_invalid( + components, +): + display_name = _get_labels( + "invalid_name", + components, + current_component_name="invalid_name", + display_name="new_name", + ) + assert display_name == "new_name" diff --git a/tests/test_generate.py b/tests/test_generate.py index 6c3ec63..fc70db9 100644 --- a/tests/test_generate.py +++ b/tests/test_generate.py @@ -377,7 +377,7 @@ def test_generator_layout_widgets(generator, index, x, y): # TODO: Split up test -def test_generator_build_screen(generator): +def test_generator_build_screen(generator, components): generator._create_widget = Mock(return_value=Mock()) generator.layout_widgets = Mock( return_value=[ @@ -392,12 +392,12 @@ def test_generator_build_screen(generator): screen_components = [Mock(), Mock(), Mock()] generator.build_widgets(screen_name, screen_components) - generator.build_groups(screen_name) + generator.build_groups(screen_name, components) generator.build_screen(screen_name) assert objectify.fromstring(str(generator.screen_)).xpath("//widget[@type='group']") -def test_build_groups_with_label(generator): +def test_build_groups_with_label(generator, components): screen_name = "motor" generator.widgets = [Mock(), Mock(), Mock()] generator._create_widget = Mock(return_value=Mock()) @@ -410,12 +410,13 @@ def test_build_groups_with_label(generator): ] ) generator._get_group_dimensions = Mock(return_value=(600, 400)) - generator.build_groups(screen_name) + + generator.build_groups(screen_name, components) xml = objectify.fromstring(str(generator.group)) assert xml.xpath("//name")[0] == "Motor Stage" -def test_build_groups(generator): +def test_build_groups(generator, components): screen_name = "test" generator.widgets = [Mock(), Mock(), Mock()] generator._create_widget = Mock(return_value=Mock()) @@ -428,7 +429,8 @@ def test_build_groups(generator): ] ) generator._get_group_dimensions = Mock(return_value=(600, 400)) - generator.build_groups(screen_name) + + generator.build_groups(screen_name, components) xml = objectify.fromstring(str(generator.group)) assert xml.xpath("//name")[0] == "test" From 07286e810e981e0da69b9a5ac36537ed2fa5bfa4 Mon Sep 17 00:00:00 2001 From: "Sode, Adedamola (DLSLtd,RAL,LSCI)" Date: Thu, 7 May 2026 08:27:09 +0000 Subject: [PATCH 3/7] patched _get_labels in generate_json --- tests/test_builder.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/tests/test_builder.py b/tests/test_builder.py index 86a5448..8435a30 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -295,7 +295,9 @@ def test_write_json_map(builder): # We don't want to access the _get_action_group function in this test @patch("techui_builder.builder._get_action_group") +@patch("techui_builder.builder._get_labels") def test_generate_json_map( + mock_get_labels, mock_get_action_group, builder_with_test_files, example_json_map, @@ -307,6 +309,7 @@ def test_generate_json_map( mock_xml = objectify.Element("action") mock_xml["file"] = "test_child_bob.bob" mock_get_action_group.return_value = mock_xml + mock_get_labels.side_effect = ["Display", "Detector"] test_json_map = builder_with_test_files._generate_json_map( screen_path.absolute(), dest_path, components @@ -316,21 +319,26 @@ def test_generate_json_map( # TODO: write this test +@patch("techui_builder.builder._get_labels") def test_generate_json_map_embedded_screen( - builder_with_test_files, example_json_map, components + mock_get_labels, builder_with_test_files, example_json_map, components ): + mock_get_labels.side_effect = ["Display", "Detector", "Embedded Display"] + screen_path = Path("tests/test_files/test_bob_embedded.bob").absolute() dest_path = Path("tests/test_files/") - test_json_map = builder_with_test_files._generate_json_map( - screen_path, dest_path, components - ) example_json_map.file = "test_bob_embedded.bob" example_json_map.children.append( JsonMap( "$(IOC)/pmacAxis.pvi.bob", display_name="Embedded Display", exists=False ) ) + + test_json_map = builder_with_test_files._generate_json_map( + screen_path, dest_path, components + ) + assert test_json_map == example_json_map @@ -396,7 +404,9 @@ def test_fix_names_json_map_recursive(builder, example_display_names_json): # We don't want to access the _get_action_group function in this test @patch("techui_builder.builder._get_action_group") +@patch("techui_builder.builder._get_labels") def test_generate_json_map_get_macros( + mock_get_labels, mock_get_action_group, builder_with_test_files, example_json_map, @@ -413,12 +423,12 @@ def test_generate_json_map_get_macros( macros = objectify.SubElement(mock_xml, "macros") # Set a macro to test macros["macro"] = "value" + mock_get_labels.side_effect = ["Display", "Detector"] mock_get_action_group.return_value = mock_xml test_json_map = builder_with_test_files._generate_json_map( screen_path, dest_path, components ) - assert test_json_map == example_json_map @@ -436,12 +446,18 @@ def test_generate_json_map_xml_parse_error( @patch("techui_builder.builder._get_action_group") +@patch("techui_builder.builder._get_labels") def test_generate_json_map_other_exception( - mock_get_action_group, builder_with_test_files, test_files, components + mock_get_labels, + mock_get_action_group, + builder_with_test_files, + test_files, + components, ): screen_path, dest_path = test_files mock_get_action_group.side_effect = Exception("Some exception") + mock_get_labels.side_effect = ["Display", "Detector"] test_json_map = builder_with_test_files._generate_json_map( screen_path, dest_path, components From b8ed10e5f3b5381e5ef138140593973110ab4f45 Mon Sep 17 00:00:00 2001 From: "Sode, Adedamola (DLSLtd,RAL,LSCI)" Date: Thu, 7 May 2026 09:03:30 +0000 Subject: [PATCH 4/7] get_labels -> _get_label --- src/techui_builder/builder.py | 10 ++++---- tests/test_builder.py | 46 +++++++++++++++++------------------ 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/techui_builder/builder.py b/src/techui_builder/builder.py index 418e3bb..e87eef3 100644 --- a/src/techui_builder/builder.py +++ b/src/techui_builder/builder.py @@ -288,7 +288,7 @@ def _generate_json_map( current_node.display_name = self._parse_display_name( root.name.text, screen_path ) - current_node.display_name = _get_labels( + current_node.display_name = _get_label( name_elem, component, current_component_name, @@ -331,7 +331,7 @@ def _generate_json_map( # Validated screen names don't get renegerated display_name = name_elem - display_name = _get_labels( + display_name = _get_label( name_elem, component, current_component_name, @@ -546,15 +546,15 @@ def _get_action_group(element: ObjectifiedElement) -> ObjectifiedElement | None: ) -def _get_labels( +def _get_label( name_elem: str | None, component: dict[str, Component], current_component_name: str | None, display_name: str | None, ) -> str | None: """ - Get display name from child labels if they exist, otherwise return name_elem - or existing display_name if name_elem is None. + Get display name from the label or child labels if they exist, otherwise return + name_elem or existing display_name if name_elem is None. """ if name_elem is not None: if name_elem in component.keys() and component[name_elem].label is not None: diff --git a/tests/test_builder.py b/tests/test_builder.py index 8435a30..bb4527f 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -12,7 +12,7 @@ from techui_builder.builder import ( JsonMap, _get_action_group, # type: ignore - _get_labels, # type: ignore + _get_label, # type: ignore _serialise_json_map, # type: ignore ) @@ -295,9 +295,9 @@ def test_write_json_map(builder): # We don't want to access the _get_action_group function in this test @patch("techui_builder.builder._get_action_group") -@patch("techui_builder.builder._get_labels") +@patch("techui_builder.builder._get_label") def test_generate_json_map( - mock_get_labels, + mock_get_label, mock_get_action_group, builder_with_test_files, example_json_map, @@ -309,7 +309,7 @@ def test_generate_json_map( mock_xml = objectify.Element("action") mock_xml["file"] = "test_child_bob.bob" mock_get_action_group.return_value = mock_xml - mock_get_labels.side_effect = ["Display", "Detector"] + mock_get_label.side_effect = ["Display", "Detector"] test_json_map = builder_with_test_files._generate_json_map( screen_path.absolute(), dest_path, components @@ -319,11 +319,11 @@ def test_generate_json_map( # TODO: write this test -@patch("techui_builder.builder._get_labels") +@patch("techui_builder.builder._get_label") def test_generate_json_map_embedded_screen( - mock_get_labels, builder_with_test_files, example_json_map, components + mock_get_label, builder_with_test_files, example_json_map, components ): - mock_get_labels.side_effect = ["Display", "Detector", "Embedded Display"] + mock_get_label.side_effect = ["Display", "Detector", "Embedded Display"] screen_path = Path("tests/test_files/test_bob_embedded.bob").absolute() dest_path = Path("tests/test_files/") @@ -404,9 +404,9 @@ def test_fix_names_json_map_recursive(builder, example_display_names_json): # We don't want to access the _get_action_group function in this test @patch("techui_builder.builder._get_action_group") -@patch("techui_builder.builder._get_labels") +@patch("techui_builder.builder._get_label") def test_generate_json_map_get_macros( - mock_get_labels, + mock_get_label, mock_get_action_group, builder_with_test_files, example_json_map, @@ -423,7 +423,7 @@ def test_generate_json_map_get_macros( macros = objectify.SubElement(mock_xml, "macros") # Set a macro to test macros["macro"] = "value" - mock_get_labels.side_effect = ["Display", "Detector"] + mock_get_label.side_effect = ["Display", "Detector"] mock_get_action_group.return_value = mock_xml test_json_map = builder_with_test_files._generate_json_map( @@ -446,9 +446,9 @@ def test_generate_json_map_xml_parse_error( @patch("techui_builder.builder._get_action_group") -@patch("techui_builder.builder._get_labels") +@patch("techui_builder.builder._get_label") def test_generate_json_map_other_exception( - mock_get_labels, + mock_get_label, mock_get_action_group, builder_with_test_files, test_files, @@ -457,7 +457,7 @@ def test_generate_json_map_other_exception( screen_path, dest_path = test_files mock_get_action_group.side_effect = Exception("Some exception") - mock_get_labels.side_effect = ["Display", "Detector"] + mock_get_label.side_effect = ["Display", "Detector"] test_json_map = builder_with_test_files._generate_json_map( screen_path, dest_path, components @@ -514,8 +514,8 @@ def test_get_action_group_no_actions_group(caplog): assert "Actions group not found" in log_output.message -def test_get_labels(components): - display_name = _get_labels( +def test_get_label(components): + display_name = _get_label( "motor", components, None, @@ -524,8 +524,8 @@ def test_get_labels(components): assert display_name == "Motor Stage" -def test_get_labels_child_labels(components): - display_name = _get_labels( +def test_get_label_child_labels(components): + display_name = _get_label( "X", components, current_component_name="motor", @@ -534,10 +534,10 @@ def test_get_labels_child_labels(components): assert display_name == "X1" -def test_get_labels_child_labels_with_name_already_pregenerated( +def test_get_label_child_labels_with_name_already_pregenerated( components, ): - display_name = _get_labels( + display_name = _get_label( "X1", components, current_component_name="motor", @@ -546,10 +546,10 @@ def test_get_labels_child_labels_with_name_already_pregenerated( assert display_name == "X1" -def test_get_labels_with_name_elem_invalid( +def test_get_label_with_name_elem_invalid( components, ): - display_name = _get_labels( + display_name = _get_label( "invalid_name", components, current_component_name=None, @@ -558,10 +558,10 @@ def test_get_labels_with_name_elem_invalid( assert display_name == "new_name" -def test_get_labels_with_current_component_name_invalid( +def test_get_label_with_current_component_name_invalid( components, ): - display_name = _get_labels( + display_name = _get_label( "invalid_name", components, current_component_name="invalid_name", From 151da8faabca37a46301cda77db07711e0da1152 Mon Sep 17 00:00:00 2001 From: "Sode, Adedamola (DLSLtd,RAL,LSCI)" Date: Thu, 7 May 2026 13:58:43 +0000 Subject: [PATCH 5/7] made _get_label a method of builder --- src/techui_builder/builder.py | 87 +++++++++++++++++------------------ tests/test_builder.py | 70 +++++++++------------------- 2 files changed, 65 insertions(+), 92 deletions(-) diff --git a/src/techui_builder/builder.py b/src/techui_builder/builder.py index e87eef3..ff5e25d 100644 --- a/src/techui_builder/builder.py +++ b/src/techui_builder/builder.py @@ -13,7 +13,7 @@ from softioc.builder import records from techui_builder.generate import Generator -from techui_builder.models import Component, Entity, TechUi +from techui_builder.models import Entity, TechUi from techui_builder.validator import Validator logger_ = logging.getLogger(__name__) @@ -261,7 +261,6 @@ def _generate_json_map( self, screen_path: Path, dest_path: Path, - component: dict[str, Component], current_component_name: str | None = None, name_elem: str | None = None, ) -> JsonMap: @@ -274,7 +273,7 @@ def _generate_json_map( ) # Get Current Component - if current_component_name is None and screen_path.stem in component: + if current_component_name is None and screen_path.stem in self.conf.components: current_component_name = screen_path.stem abs_path = screen_path.absolute() @@ -288,9 +287,8 @@ def _generate_json_map( current_node.display_name = self._parse_display_name( root.name.text, screen_path ) - current_node.display_name = _get_label( + current_node.display_name = self._get_label( name_elem, - component, current_component_name, current_node.display_name, ) @@ -331,9 +329,8 @@ def _generate_json_map( # Validated screen names don't get renegerated display_name = name_elem - display_name = _get_label( + display_name = self._get_label( name_elem, - component, current_component_name, display_name, ) @@ -357,7 +354,6 @@ def _generate_json_map( child_node = self._generate_json_map( next_file_path, dest_path, - component, current_component_name=current_component_name, name_elem=name_elem, ) @@ -377,10 +373,43 @@ def _generate_json_map( except Exception as e: current_node.error = str(e) - self._fix_names_json_map(current_node, component) + self._fix_names_json_map(current_node) return current_node + def _get_label( + self, + name_elem: str | None, + current_component_name: str | None, + display_name: str | None, + ) -> str | None: + """ + Get display name from the label or child labels if they exist, otherwise return + name_elem or existing display_name if name_elem is None. + """ + component = self.conf.components + if name_elem is not None: + if name_elem in component.keys() and component[name_elem].label is not None: + display_name = component[name_elem].label + elif ( + current_component_name is not None + and (current_component_name in component.keys()) + and (component[current_component_name].child_labels is not None) + ): + child_labels = component[current_component_name].child_labels + if child_labels is not None: + # Because name_elem is initially grabbed from + # the .bob file, the generated .bobfile might have + # already propagated the child label from techui.yaml + if name_elem in child_labels.values(): + display_name = name_elem + # In the case of screens not regenerated, such as validated screens, + # the name text will not be updated to the childlabel,so we check + # keys solely for generating the json_map from the top level .bob. + elif name_elem in child_labels: + display_name = child_labels[name_elem] + return display_name + def _extract_action_button_file_from_embedded( self, file_elem: ObjectifiedElement, dest_path: Path ) -> ObjectifiedElement: @@ -436,7 +465,8 @@ def _parse_display_name(self, name: str | None, file_path: Path) -> str | None: return None def _fix_names_json_map( - self, node: JsonMap, components: dict[str, Component] + self, + node: JsonMap, ) -> None: """Recursively fix duplicate display names in children""" if not node.children: @@ -463,7 +493,7 @@ def _fix_names_json_map( # recursively fix children for child in node.children: - self._fix_names_json_map(child, components) + self._fix_names_json_map(child) def write_json_map( self, @@ -479,7 +509,7 @@ def write_json_map( f"Cannot generate json map for {synoptic}. Has it been generated?" ) - map = self._generate_json_map(synoptic, dest, self.conf.components) + map = self._generate_json_map(synoptic, dest) with open(dest.joinpath("JsonMap.json"), "w") as f: f.write( json.dumps(map, indent=4, default=lambda o: _serialise_json_map(o)) @@ -544,36 +574,3 @@ def _get_action_group(element: ObjectifiedElement) -> ObjectifiedElement | None: f"Actions group not found in component [bold]{name}[/bold] on " f"[bold]{parent_name}[/bold]" ) - - -def _get_label( - name_elem: str | None, - component: dict[str, Component], - current_component_name: str | None, - display_name: str | None, -) -> str | None: - """ - Get display name from the label or child labels if they exist, otherwise return - name_elem or existing display_name if name_elem is None. - """ - if name_elem is not None: - if name_elem in component.keys() and component[name_elem].label is not None: - display_name = component[name_elem].label - elif ( - current_component_name is not None - and (current_component_name in component.keys()) - and (component[current_component_name].child_labels is not None) - ): - child_labels = component[current_component_name].child_labels - if child_labels is not None: - # Because name_elem is initially - # grabbed from the .bob file, the generated .bob - # file might have already propagated the child label from techui.yaml - if name_elem in child_labels.values(): - display_name = name_elem - # In the case of screens not regenerated, such as validated screens, - # the name text will not be updated to the childlabel,so we check the - # keys solely for generating the json_map from the top level .bob file - elif name_elem in child_labels: - display_name = child_labels[name_elem] - return display_name diff --git a/tests/test_builder.py b/tests/test_builder.py index bb4527f..a0ee0ad 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -12,7 +12,6 @@ from techui_builder.builder import ( JsonMap, _get_action_group, # type: ignore - _get_label, # type: ignore _serialise_json_map, # type: ignore ) @@ -295,35 +294,33 @@ def test_write_json_map(builder): # We don't want to access the _get_action_group function in this test @patch("techui_builder.builder._get_action_group") -@patch("techui_builder.builder._get_label") def test_generate_json_map( - mock_get_label, mock_get_action_group, builder_with_test_files, example_json_map, test_files, - components, ): screen_path, dest_path = test_files mock_xml = objectify.Element("action") mock_xml["file"] = "test_child_bob.bob" mock_get_action_group.return_value = mock_xml - mock_get_label.side_effect = ["Display", "Detector"] + builder_with_test_files._get_label = Mock(side_effect=["Display", "Detector"]) test_json_map = builder_with_test_files._generate_json_map( - screen_path.absolute(), dest_path, components + screen_path.absolute(), dest_path ) assert test_json_map == example_json_map # TODO: write this test -@patch("techui_builder.builder._get_label") def test_generate_json_map_embedded_screen( - mock_get_label, builder_with_test_files, example_json_map, components + builder_with_test_files, example_json_map, components ): - mock_get_label.side_effect = ["Display", "Detector", "Embedded Display"] + builder_with_test_files._get_label = Mock( + side_effect=["Display", "Detector", "Embedded Display"] + ) screen_path = Path("tests/test_files/test_bob_embedded.bob").absolute() dest_path = Path("tests/test_files/") @@ -397,21 +394,18 @@ def test_fix_names_json_map_recursive(builder, example_display_names_json): test_display_names_json.children.append(test_display_names_json_dev1) test_display_names_json.children.append(test_display_names_json_dev2) - builder._fix_names_json_map(test_display_names_json, builder.conf.components) + builder._fix_names_json_map(test_display_names_json) assert test_display_names_json == example_display_names_json # We don't want to access the _get_action_group function in this test @patch("techui_builder.builder._get_action_group") -@patch("techui_builder.builder._get_label") def test_generate_json_map_get_macros( - mock_get_label, mock_get_action_group, builder_with_test_files, example_json_map, test_files, - components, ): screen_path, dest_path = test_files @@ -423,45 +417,34 @@ def test_generate_json_map_get_macros( macros = objectify.SubElement(mock_xml, "macros") # Set a macro to test macros["macro"] = "value" - mock_get_label.side_effect = ["Display", "Detector"] + builder_with_test_files._get_label = Mock(side_effect=["Display", "Detector"]) mock_get_action_group.return_value = mock_xml - test_json_map = builder_with_test_files._generate_json_map( - screen_path, dest_path, components - ) + test_json_map = builder_with_test_files._generate_json_map(screen_path, dest_path) assert test_json_map == example_json_map -def test_generate_json_map_xml_parse_error( - builder_with_test_files, test_files, components -): +def test_generate_json_map_xml_parse_error(builder_with_test_files, test_files): screen_path = Path("tests/test_files/test_bob_bad.bob").absolute() _, dest_path = test_files - test_json_map = builder_with_test_files._generate_json_map( - screen_path, dest_path, components - ) + test_json_map = builder_with_test_files._generate_json_map(screen_path, dest_path) assert test_json_map.error.startswith("XML parse error:") @patch("techui_builder.builder._get_action_group") -@patch("techui_builder.builder._get_label") def test_generate_json_map_other_exception( - mock_get_label, mock_get_action_group, builder_with_test_files, test_files, - components, ): screen_path, dest_path = test_files mock_get_action_group.side_effect = Exception("Some exception") - mock_get_label.side_effect = ["Display", "Detector"] + builder_with_test_files._get_label = Mock(side_effect=["Display", "Detector"]) - test_json_map = builder_with_test_files._generate_json_map( - screen_path, dest_path, components - ) + test_json_map = builder_with_test_files._generate_json_map(screen_path, dest_path) assert test_json_map.error != "" @@ -514,32 +497,27 @@ def test_get_action_group_no_actions_group(caplog): assert "Actions group not found" in log_output.message -def test_get_label(components): - display_name = _get_label( +def test_get_label(builder_with_test_files): + display_name = builder_with_test_files._get_label( "motor", - components, None, None, ) assert display_name == "Motor Stage" -def test_get_label_child_labels(components): - display_name = _get_label( +def test_get_label_child_labels(builder_with_test_files): + display_name = builder_with_test_files._get_label( "X", - components, current_component_name="motor", display_name="X", ) assert display_name == "X1" -def test_get_label_child_labels_with_name_already_pregenerated( - components, -): - display_name = _get_label( +def test_get_label_child_labels_with_name_already_pregenerated(builder_with_test_files): + display_name = builder_with_test_files._get_label( "X1", - components, current_component_name="motor", display_name="X", ) @@ -547,11 +525,10 @@ def test_get_label_child_labels_with_name_already_pregenerated( def test_get_label_with_name_elem_invalid( - components, + builder_with_test_files, ): - display_name = _get_label( + display_name = builder_with_test_files._get_label( "invalid_name", - components, current_component_name=None, display_name="new_name", ) @@ -559,11 +536,10 @@ def test_get_label_with_name_elem_invalid( def test_get_label_with_current_component_name_invalid( - components, + builder_with_test_files, ): - display_name = _get_label( + display_name = builder_with_test_files._get_label( "invalid_name", - components, current_component_name="invalid_name", display_name="new_name", ) From cf63cf99dd227390cfe8f029be7f2e8c3939377c Mon Sep 17 00:00:00 2001 From: "Sode, Adedamola (DLSLtd,RAL,LSCI)" Date: Thu, 7 May 2026 14:03:03 +0000 Subject: [PATCH 6/7] method name change: _get_label -> _get_component_name --- src/techui_builder/builder.py | 6 +++--- tests/test_builder.py | 36 +++++++++++++++++++++-------------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/techui_builder/builder.py b/src/techui_builder/builder.py index ff5e25d..a59431e 100644 --- a/src/techui_builder/builder.py +++ b/src/techui_builder/builder.py @@ -287,7 +287,7 @@ def _generate_json_map( current_node.display_name = self._parse_display_name( root.name.text, screen_path ) - current_node.display_name = self._get_label( + current_node.display_name = self._get_component_name( name_elem, current_component_name, current_node.display_name, @@ -329,7 +329,7 @@ def _generate_json_map( # Validated screen names don't get renegerated display_name = name_elem - display_name = self._get_label( + display_name = self._get_component_name( name_elem, current_component_name, display_name, @@ -377,7 +377,7 @@ def _generate_json_map( return current_node - def _get_label( + def _get_component_name( self, name_elem: str | None, current_component_name: str | None, diff --git a/tests/test_builder.py b/tests/test_builder.py index a0ee0ad..d2b7eec 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -305,7 +305,9 @@ def test_generate_json_map( mock_xml = objectify.Element("action") mock_xml["file"] = "test_child_bob.bob" mock_get_action_group.return_value = mock_xml - builder_with_test_files._get_label = Mock(side_effect=["Display", "Detector"]) + builder_with_test_files._get_component_name = Mock( + side_effect=["Display", "Detector"] + ) test_json_map = builder_with_test_files._generate_json_map( screen_path.absolute(), dest_path @@ -318,7 +320,7 @@ def test_generate_json_map( def test_generate_json_map_embedded_screen( builder_with_test_files, example_json_map, components ): - builder_with_test_files._get_label = Mock( + builder_with_test_files._get_component_name = Mock( side_effect=["Display", "Detector", "Embedded Display"] ) @@ -417,7 +419,9 @@ def test_generate_json_map_get_macros( macros = objectify.SubElement(mock_xml, "macros") # Set a macro to test macros["macro"] = "value" - builder_with_test_files._get_label = Mock(side_effect=["Display", "Detector"]) + builder_with_test_files._get_component_name = Mock( + side_effect=["Display", "Detector"] + ) mock_get_action_group.return_value = mock_xml test_json_map = builder_with_test_files._generate_json_map(screen_path, dest_path) @@ -442,7 +446,9 @@ def test_generate_json_map_other_exception( screen_path, dest_path = test_files mock_get_action_group.side_effect = Exception("Some exception") - builder_with_test_files._get_label = Mock(side_effect=["Display", "Detector"]) + builder_with_test_files._get_component_name = Mock( + side_effect=["Display", "Detector"] + ) test_json_map = builder_with_test_files._generate_json_map(screen_path, dest_path) @@ -497,8 +503,8 @@ def test_get_action_group_no_actions_group(caplog): assert "Actions group not found" in log_output.message -def test_get_label(builder_with_test_files): - display_name = builder_with_test_files._get_label( +def test_get_component_name(builder_with_test_files): + display_name = builder_with_test_files._get_component_name( "motor", None, None, @@ -506,8 +512,8 @@ def test_get_label(builder_with_test_files): assert display_name == "Motor Stage" -def test_get_label_child_labels(builder_with_test_files): - display_name = builder_with_test_files._get_label( +def test_get_component_name_child_labels(builder_with_test_files): + display_name = builder_with_test_files._get_component_name( "X", current_component_name="motor", display_name="X", @@ -515,8 +521,10 @@ def test_get_label_child_labels(builder_with_test_files): assert display_name == "X1" -def test_get_label_child_labels_with_name_already_pregenerated(builder_with_test_files): - display_name = builder_with_test_files._get_label( +def test_get_component_name_child_labels_with_name_already_pregenerated( + builder_with_test_files, +): + display_name = builder_with_test_files._get_component_name( "X1", current_component_name="motor", display_name="X", @@ -524,10 +532,10 @@ def test_get_label_child_labels_with_name_already_pregenerated(builder_with_test assert display_name == "X1" -def test_get_label_with_name_elem_invalid( +def test_get_component_name_with_name_elem_invalid( builder_with_test_files, ): - display_name = builder_with_test_files._get_label( + display_name = builder_with_test_files._get_component_name( "invalid_name", current_component_name=None, display_name="new_name", @@ -535,10 +543,10 @@ def test_get_label_with_name_elem_invalid( assert display_name == "new_name" -def test_get_label_with_current_component_name_invalid( +def test_get_component_name_with_current_component_name_invalid( builder_with_test_files, ): - display_name = builder_with_test_files._get_label( + display_name = builder_with_test_files._get_component_name( "invalid_name", current_component_name="invalid_name", display_name="new_name", From 392f50b517d76490e592bd0cd6637fad57d5cb4d Mon Sep 17 00:00:00 2001 From: "Sode, Adedamola (DLSLtd,RAL,LSCI)" Date: Thu, 7 May 2026 14:06:51 +0000 Subject: [PATCH 7/7] method name change: _get_component_name -> _get_component_label --- src/techui_builder/builder.py | 6 +++--- tests/test_builder.py | 28 ++++++++++++++-------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/techui_builder/builder.py b/src/techui_builder/builder.py index a59431e..fdfbdac 100644 --- a/src/techui_builder/builder.py +++ b/src/techui_builder/builder.py @@ -287,7 +287,7 @@ def _generate_json_map( current_node.display_name = self._parse_display_name( root.name.text, screen_path ) - current_node.display_name = self._get_component_name( + current_node.display_name = self._get_component_label( name_elem, current_component_name, current_node.display_name, @@ -329,7 +329,7 @@ def _generate_json_map( # Validated screen names don't get renegerated display_name = name_elem - display_name = self._get_component_name( + display_name = self._get_component_label( name_elem, current_component_name, display_name, @@ -377,7 +377,7 @@ def _generate_json_map( return current_node - def _get_component_name( + def _get_component_label( self, name_elem: str | None, current_component_name: str | None, diff --git a/tests/test_builder.py b/tests/test_builder.py index d2b7eec..24375b0 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -305,7 +305,7 @@ def test_generate_json_map( mock_xml = objectify.Element("action") mock_xml["file"] = "test_child_bob.bob" mock_get_action_group.return_value = mock_xml - builder_with_test_files._get_component_name = Mock( + builder_with_test_files._get_component_label = Mock( side_effect=["Display", "Detector"] ) @@ -320,7 +320,7 @@ def test_generate_json_map( def test_generate_json_map_embedded_screen( builder_with_test_files, example_json_map, components ): - builder_with_test_files._get_component_name = Mock( + builder_with_test_files._get_component_label = Mock( side_effect=["Display", "Detector", "Embedded Display"] ) @@ -419,7 +419,7 @@ def test_generate_json_map_get_macros( macros = objectify.SubElement(mock_xml, "macros") # Set a macro to test macros["macro"] = "value" - builder_with_test_files._get_component_name = Mock( + builder_with_test_files._get_component_label = Mock( side_effect=["Display", "Detector"] ) mock_get_action_group.return_value = mock_xml @@ -446,7 +446,7 @@ def test_generate_json_map_other_exception( screen_path, dest_path = test_files mock_get_action_group.side_effect = Exception("Some exception") - builder_with_test_files._get_component_name = Mock( + builder_with_test_files._get_component_label = Mock( side_effect=["Display", "Detector"] ) @@ -503,8 +503,8 @@ def test_get_action_group_no_actions_group(caplog): assert "Actions group not found" in log_output.message -def test_get_component_name(builder_with_test_files): - display_name = builder_with_test_files._get_component_name( +def test_get_component_label(builder_with_test_files): + display_name = builder_with_test_files._get_component_label( "motor", None, None, @@ -512,8 +512,8 @@ def test_get_component_name(builder_with_test_files): assert display_name == "Motor Stage" -def test_get_component_name_child_labels(builder_with_test_files): - display_name = builder_with_test_files._get_component_name( +def test_get_component_label_child_labels(builder_with_test_files): + display_name = builder_with_test_files._get_component_label( "X", current_component_name="motor", display_name="X", @@ -521,10 +521,10 @@ def test_get_component_name_child_labels(builder_with_test_files): assert display_name == "X1" -def test_get_component_name_child_labels_with_name_already_pregenerated( +def test_get_component_label_child_labels_with_name_already_pregenerated( builder_with_test_files, ): - display_name = builder_with_test_files._get_component_name( + display_name = builder_with_test_files._get_component_label( "X1", current_component_name="motor", display_name="X", @@ -532,10 +532,10 @@ def test_get_component_name_child_labels_with_name_already_pregenerated( assert display_name == "X1" -def test_get_component_name_with_name_elem_invalid( +def test_get_component_label_with_name_elem_invalid( builder_with_test_files, ): - display_name = builder_with_test_files._get_component_name( + display_name = builder_with_test_files._get_component_label( "invalid_name", current_component_name=None, display_name="new_name", @@ -543,10 +543,10 @@ def test_get_component_name_with_name_elem_invalid( assert display_name == "new_name" -def test_get_component_name_with_current_component_name_invalid( +def test_get_component_label_with_current_component_name_invalid( builder_with_test_files, ): - display_name = builder_with_test_files._get_component_name( + display_name = builder_with_test_files._get_component_label( "invalid_name", current_component_name="invalid_name", display_name="new_name",