Skip to content

Feature: Unify name/wildcard access and miss-behavior across all holders #433

Description

@gupichon

Description, motivation and use case

Name-based lookup exists in at least six different places on the ElementHolder and array side (ElementHolder.__getitem__/.find_elements(), GenericElementHolder.get(), GenericArrayHolder.get()/__getitem__/__getattr__, ElementArray.__getitem__, RFHolder.get(), ToolHolder.get()), each with its own behavior: what counts as a wildcard, whether re: regex is supported, and what happens when nothing matches (None, an exception, AttributeError, or a silently empty or unchecked result).

This was found while adding ToolHolder (#375). sr.live.bpm["BPM_001"] raises TypeError since singular holders have no __getitem__, while sr.live["BPM_001"] and sr.live.bpms["BPM_001"] both "work", with different, disagreeing semantics from each other.

Proposed solution

Define one shared name and wildcard resolution convention, then apply it consistently:

  • Add __getitem__ to GenericElementHolder, backing .magnet, .bpm, .combined_function_magnet, .serialized_magnet, with the same wildcard syntax as ElementHolder.__getitem__.
  • Reconcile GenericArrayHolder.__getitem__, currently searching individual element names, with GenericArrayHolder.get(name), which searches registered array names. Decide, and document, which namespace [] should query, since they currently disagree silently.
  • Extend RFHolder and ToolHolder with the same [] convention.
  • Uniform miss behavior, decided: a literal, exact name, no wildcard character, that does not exist must raise PyAMLException everywhere. That covers ElementHolder.__getitem__, which currently returns None, GenericElementHolder, GenericArrayHolder, RFHolder and ToolHolder once they gain __getitem__, and find_elements(), which currently returns an unchecked literal name with no existence check. A wildcard search that matches nothing is not an error and keeps returning an empty collection, as today.
  • Wildcard trigger, decided: *, ? or [ anywhere in the key means this is an fnmatch pattern, everywhere, including find_elements(), which today only checks for * and ? and silently treats a string containing only [...], for example "BPM_00[1-5]", as a literal name.
  • Regex support, dropped. Team feedback (see comments) is that fnmatch already covers the vast majority of real-world naming patterns, and re: would only reintroduce the ambiguity and collision risk discussed above for little practical benefit. [] and find_elements() stay fnmatch only, no re: prefix anywhere on this side of the API. YellowPages keeps its own re: support, unaffected, tracked separately under Align the output of the yellow pages with the content of the accelerator #365.
  • Case sensitivity, decided: use fnmatch.fnmatchcase() instead of fnmatch.fnmatch() everywhere a name gets matched against an fnmatch pattern. fnmatch.fnmatch() normalizes both the name and the pattern through os.path.normcase() first, which makes matching case-insensitive on Windows and case-sensitive on Linux, so the same configuration can match differently depending on the host OS. fnmatchcase() skips that normalization and stays case-sensitive everywhere. This applies to every existing fnmatch call site, ElementHolder.__getitem__, ElementArray.__getitem__, find_elements(), not only to the new ones this issue adds, GenericElementHolder.__getitem__, RFHolder.__getitem__, ToolHolder.__getitem__. YellowPages has the same case-sensitivity gap today, it is out of scope here but worth flagging for Align the output of the yellow pages with the content of the accelerator #365 too.

Describe alternatives you've considered

Leaving each holder with its own convention avoids behavior changes but keeps the current footguns, for example .bpms["BPMS"] silently querying the wrong namespace, find_elements("X[1-3]") silently treated as literal, or sr.live["UNKNOWN"] silently returning None instead of surfacing a typo.

Additional context

Parent issue, ElementHolder API refurbishment #199.

Found while implementing #375. This is a breaking change for any caller relying on ElementHolder.__getitem__'s current None on miss behavior, documented today at element_holder.py:498, missing = sr.live["UNKNOWN"] returning None. It should be called out in the changelog or release notes.

YellowPages is intentionally out of scope for this issue. #365 already covers aligning its categories with the ElementHolder sub-holders, and should be the one place deciding its own re: and fnmatch parity too, its scope may need to grow slightly to cover that, rather than splitting the discussion across two issues.

Checklist

  • I've assigned this issue to a project
  • I've @-mentioned relevant people

Activity

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions