-
Notifications
You must be signed in to change notification settings - Fork 1
Added label feature for the techui.yaml #216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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"} | ||
|
|
@@ -215,7 +217,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: | ||
|
|
@@ -246,7 +254,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 | ||
|
|
@@ -255,6 +270,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: | ||
|
|
@@ -266,7 +285,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 <widget> elements | ||
| widgets = [ | ||
| w | ||
|
|
@@ -302,6 +326,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 "") | ||
|
|
||
|
|
@@ -310,15 +343,21 @@ 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) | ||
|
|
||
| # 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) | ||
|
|
@@ -335,7 +374,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 | ||
|
|
||
|
|
@@ -393,7 +432,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 | ||
|
|
@@ -407,6 +448,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']})" | ||
|
|
@@ -418,7 +460,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, | ||
|
|
@@ -434,7 +476,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)) | ||
|
|
@@ -499,3 +541,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 ( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the indentation here is wrong maybe? Also can you write a test for if |
||
| 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 | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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, | ||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small gripe, but the indentation here is wrong 😅 |
||||||||||||||||||
| # 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(":")] | ||||||||||||||||||
|
Comment on lines
+181
to
+185
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As this is reused, it would be tidier to put it in a temporary variable, e.g.
Suggested change
|
||||||||||||||||||
| 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.P.lower()}") | ||||||||||||||||||
|
|
@@ -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 | ||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||
| 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, | ||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -109,10 +109,10 @@ def example_display_names_json(): | |
|
|
||
|
|
||
| @pytest.fixture | ||
| def generator(): | ||
| def generator(builder: Builder): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer we didn't pass the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since I am trying to use the test-services techui-yaml, is it fine if I have the components fixture take in builder instead? Since that's how it's also done in the actual codebase? |
||
| 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 | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you write a test for if
name_elemexists but it isn't incomponent.keys()?