Skip to content

ENH: add template support#150

Open
tangkong wants to merge 24 commits intopcdshub:masterfrom
tangkong:enh_templates
Open

ENH: add template support#150
tangkong wants to merge 24 commits intopcdshub:masterfrom
tangkong:enh_templates

Conversation

@tangkong
Copy link
Contributor

@tangkong tangkong commented Feb 14, 2026

Description

  • Adds template data type, which references an existing collection and placeholder substitutions
  • Adds widgets for creating and filling placeholders in a template.
  • Adds Template-aware bits and bobs in other parts of the GUI app

There's still a variety of things to do here:

  • consider {{}} in CA requests, seems to bait out caproto errors?
  • UI/UX: consider creating template alone, without base collection?
  • Fix the poll model to actually read the templated PVs, and handle disconnected PVs better

Motivation and Context

One of the big asks is to be able to template Collections.

Some relatively big decisions that were made:

  • Placeholders are any string surrounded by "{{" and "}}".
    • we ignore the possibility that we actually want to use these characters in a name/description/pv
  • We only find and replace strings. We are intentionally ignoring status/severity/timestamp etc.
  • The template page determines what is a placeholder and replacement by simple substring detection

How Has This Been Tested?

Interactively for now

Where Has This Been Documented?

This PR

image image

Pre-merge checklist

  • Code works interactively
  • Code follows the style guide
  • Code contains descriptive docstrings, including context and API
  • New/changed functions and methods are covered in the test suite where possible
  • Test suite passes locally
  • Test suite passes on GitHub Actions
  • Ran docs/pre-release-notes.sh and created a pre-release documentation page
  • Pre-release docs include context, functional descriptions, and contributors as appropriate

…and filling placeholders. Attempts helpful highlighting
…ry contains placeholders (look past title/desc), add swap mode button text, filter for incomplete placeholders
…ements itself. This enables better placeholder detection
@tangkong tangkong marked this pull request as ready for review February 19, 2026 22:06
@tangkong tangkong requested a review from a team as a code owner February 19, 2026 22:06
Comment on lines +195 to +202
self.convert_template_button.show()


class SnapshotPage(NestablePage[Snapshot]):

def setup_ui(self):
super().setup_ui()
self.convert_template_button.hide()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick confusion for me: what's up with the on-start show in some pages and hide in others for this button? I'd assume either they'd all have the same default state from the ui file (shown?) or the correct state in the ui file, so at least one of these would have to be redundant

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, one of them is redundant. Maybe my past self was just wanting to be explicit about it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, no need to dwell on it I guess

Templating helpers. Widgets for creating and filling templates.

TODO:
- consider {{}} in CA requests, seems to bait out caproto errors?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is because this is part of the syntax for getting many PVs at once

$ caget IM{2..5}K{2,4}:PPM:MMS
IM2K2:PPM:MMS                  -8.49755
IM2K4:PPM:MMS                  -8.6066
IM3K2:PPM:MMS                  -9.6155
IM3K4:PPM:MMS                  -5.8363
IM4K2:PPM:MMS                  -10.8074
IM4K4:PPM:MMS                  -9.2909
IM5K2:PPM:MMS                  -72.035
IM5K4:PPM:MMS                  -5.1274

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely knew this at one point and forgot. This opens the door to the idea that maybe people might want to specify pvs via this multi-pv syntax... I definitely don't support that here yet

logger = logging.getLogger(__name__)


class FillColors(Enum):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuck out to me as a nifty helper class

self.remove_button.hide()


class HighlightProxyModel(QtCore.QIdentityProxyModel):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QIdentityProxyModel was a new one for me- seems useful

NestableHeader.DESCRIPTION
):
break
if f"{{{{{placeholder}}}}}" not in val:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Food for thought: this interaction between f strings and multiple brackets suggests to me that a different placeholder format should have been used- probably one not natively included in python syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, I'm open to suggestions here. I wanted something that was unlikely to show up in an EPICS PV, and curly braces seem to conflict with built-in CA syntax. "<>" and "[]" also seem like valid characters. Maybe something non-alphanumeric?

  • %% * %%
  • !! * !!

Comment on lines +368 to +372
- These widgets are "placeholder_*"

Right panel ("Template Substitutions") is for replacing templates with
new desired text
- These widgets are "substitute_*"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DOC: perhaps these widget naming schemes are out of date- I see stuff like *_placeholder below, for example- maybe should be removed or corrected?

@ZLLentz
Copy link
Member

ZLLentz commented Feb 20, 2026

Code looks super reasonable- I'm going to do interactive testing for this one because the screenshots look good

@ZLLentz
Copy link
Member

ZLLentz commented Feb 20, 2026

I tried to open this with

zlentz@psbuild-rocky9-01:~/github/superscore(enh_templates -)$ export SUPERSCORE_CFG=/cds/home/z/zlentz/github/superscore/superscore/tests/config.cfg
zlentz@psbuild-rocky9-01:~/github/superscore(enh_templates -)$ python -m superscore ui

but it didn't quite work when I went to fill the template in the sample collection

  File "/cds/home/z/zlentz/github/superscore/superscore/widgets/page/entry.py", line 173, in convert_to_template
    window.open_page(template)
  File "/cds/home/z/zlentz/github/superscore/superscore/widgets/window.py", line 147, in open_page
    fresh_entry = self.client.get_entry(entry.uuid)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/cds/home/z/zlentz/github/superscore/superscore/client.py", line 222, in get_entry
    entry = self.backend.get_entry(uuid)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/cds/home/z/zlentz/github/superscore/superscore/backends/filestore.py", line 265, in get_entry
    raise EntryNotFoundError(f"Entry {uuid} could not be found")
EntryNotFoundError: Entry 76555942-3c6a-493a-9d9c-b2b6267a7afd could not be found

am I doing something silly?

@tangkong
Copy link
Contributor Author

Oh whoops apparently I left out a fix I made to allow opening entries that don't yet exist in the backend. That's what I get for accumulating a bunch of minor logging fixes, I end up missing stuff in the working area

@tangkong
Copy link
Contributor Author

I've addressed the bugs Zach pointed out during our live QA session. These included:

  • Saving the template-filled collection now updates the main window tree view
  • Saving the template-filled collection now no saves the children to the root view
  • Templates now can be fully swapped to UUIDs and serialized (by modifying type hints)
  • Catch an occasional segfault (dereferenced child pointer in tree view)

Copy link
Member

@ZLLentz ZLLentz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving, but I'll let this sit dormant/unmerged for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants