Skip to content

Unify name/wildcard/regex lookup and miss-behavior across holders - #449

Open
gupichon wants to merge 2 commits into
199-elementholder-api-refurbishmentfrom
433-feature-unify-namewildcard-access-and-miss-behavior-across-all-holders
Open

gupichon wants to merge 2 commits into
199-elementholder-api-refurbishmentfrom
433-feature-unify-namewildcard-access-and-miss-behavior-across-all-holders

Conversation

@gupichon

Copy link
Copy Markdown
Member

Description

Name-based lookup (__getitem__, .get(), find_elements()) was implemented independently in at least six places across ElementHolder and the holder/array hierarchy, each with its own idea of what counts as a wildcard, whether re: regex is supported, and what happens on a miss (None, an exception, AttributeError, or a silent empty result). This surfaced while adding ToolHolder (#375): sr.live.bpm["BPM_001"] raised TypeError (no __getitem__ on singular holders) while sr.live["BPM_001"] and sr.live.bpms["BPM_001"] both "worked" with disagreeing semantics. This PR defines one shared name/wildcard resolution convention (pyaml/common/name_matching.py) and applies it consistently to every holder and array.

Related Issue

Features/issues described there are:

  • new feature: add __getitem__ to GenericElementHolder, RFHolder, RFTransmitterHolder, ToolHolder, and DiagnosticHolder — implemented by routing all of them, plus the existing ElementHolder.__getitem__/find_elements() and ElementArray.__getitem__/GenericArrayHolder.__getitem__, through one shared resolver (resolve_names()/is_wildcard()) instead of each reimplementing its own rules.
  • bugfix: fnmatch.fnmatch() normalizes through os.path.normcase(), so the same wildcard pattern matched case-insensitively on Windows and case-sensitively on Linux CI — switched every call site to fnmatch.fnmatchcase().
  • bugfix: an invalid re:-prefixed regex used to leak a raw re.error; it now raises PyAMLException like every other lookup failure.
  • decision, not code: GenericArrayHolder.__getitem__ (e.g. magnets["QF1*"]) keeps searching individual element names, not registered array/family names — this is Feature: Complete magnet holder navigation #373's recent, deliberately tested behavior. Documented explicitly instead of unifying it with .get(name)'s separate array/family-name namespace, since unifying would revert Feature: Complete magnet holder navigation #373.
  • scope addition beyond the issue text: also added __getitem__ to DiagnosticHolder and RFTransmitterHolder, which have the exact same shape as RFHolder/ToolHolder and didn't exist yet when the issue was written (they landed with Feature: Add a public diagnostic holder #372 and Feature: Add a public tuning and measurement tool holder #375).
  • scope addition, from thread discussion: regex (re: prefix) is kept, reversing what the issue currently says, and extended everywhere the resolver is used, not just find_elements(). A list/tuple of patterns (literal, wildcard, or regex, mixed) is also now accepted everywhere a single pattern is, unioned and de-duplicated.

Changes to existing functionality

  • ElementHolder.__getitem__: a literal exact name with no match now raises PyAMLException instead of returning None. Breaking change — any caller relying on the old None-on-miss must now catch PyAMLException or check membership first.
  • ElementArray.__getitem__ / GenericArrayHolder.__getitem__: same literal-miss fix (raises instead of silently returning an empty array); wildcard trigger now also includes [, not just */?; case-sensitive matching via fnmatchcase; gains re: and list-of-patterns support. A wildcard/regex pattern matching nothing is unaffected, still an empty collection, not an error.
  • ElementHolder.find_elements(): reimplemented as a thin wrapper around the shared resolver, same public behavior plus the [-trigger, case-sensitivity, and error-wrapping fixes above, and now also accepts a list of patterns directly.

Testing

The following tests (compatible with pytest) were added:

  • tests/common/test_name_matching.py: the shared resolver directly — literal hit/miss, wildcard hit/empty, regex hit/empty/invalid, case sensitivity, list-of-patterns (union/dedup/order, one missing literal raises)
  • tests/common/test_element_holder_collection.py: ElementHolder.__getitem__ / find_elements() literal-miss-raises, [-as-wildcard, re:, list-of-patterns, case sensitivity, plus new tests combining wildcard/regex/list selections with &/|/- set operations (non-trivial intersection/union/difference, and the empty-result-stays-a-plain-list quirk under the new selection styles)
  • tests/arrays/test_array_selection_types.py: ElementArray.__getitem__ literal-miss-raises, bracket-only wildcard, regex, list-of-patterns, case sensitivity
  • tests/common/test_array_holder_navigation.py: GenericElementHolder.__getitem__ via .magnet, .combined_function_magnet, .serialized_magnet; a realistic example reproducing the QForTest YAML exclusion family via [] selection and -
  • tests/diagnostics/test_diagnostic_accessors.py: DiagnosticHolder.__getitem__ and .diagnostic.bpm[...]
  • tests/rf/test_rf_holder.py (new): RFHolder/RFTransmitterHolder.__getitem__
  • tests/tuning_tools/test_tool_holder.py (new): ToolHolder.__getitem__

Verify that your checklist complies with the project

  • New and existing unit tests pass locally (pytest tests -n auto: 389 passed, 6 skipped; one unrelated pre-existing flake, test_tuning_orm, is a Windows temp-file PermissionError unconnected to this change)
  • Tests were added to prove that all features/changes are effective
  • The code is commented where appropriate
  • Any existing features are not broken, aside from the explicit, documented breaking change above (literal-miss now raises instead of returning None)

@gupichon
gupichon requested a review from GamelinAl September 18, 2026 15:21
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