Skip to content

ElementHolder.get() resolves named arrays of any type; drop supersede… - #462

Merged
gupichon merged 2 commits into
199-elementholder-api-refurbishmentfrom
elementholder-api-refurbishment-api-cleaning
Oct 2, 2026
Merged

gupichon merged 2 commits into
199-elementholder-api-refurbishmentfrom
elementholder-api-refurbishment-api-cleaning

Conversation

@gupichon

@gupichon gupichon commented Oct 1, 2026

Copy link
Copy Markdown
Member

Addresses the last round of changes requested by @GamelinAl on PR #430 before it can be approved:

  1. ElementHolder.get() did not support named-array lookup the way every typed sub-holder already does (magnets.get(name), tool.get(name), diagnostic.get(name), ...). Only the legacy get_elements(name) worked, and even that was narrower than it looked: it only searched the generic element-array store, never magnet/BPM/CFM/serialized-magnet arrays.
  2. The API needed cleaning up by removing get_element, get_elements, get_all_elements, the get_*_tuning family, and get_betatron_tune_monitor, now that their typed replacements (holder[name], holder.get(name), holder.tool.get(name), holder.diagnostic.get(name)) exist.

Related Issue

Features/issues described there are:

  • bugfix: ElementHolder.get(name=None) now delegates to the existing _get_array resolver, which already searches all five array stores (BPM, magnet, CFM magnet, serialized-magnet, generic element) - so sr.live.get("CELL08") works regardless of which typed array "CELL08" is, instead of being limited to generic element arrays.
  • cleanup: removed get_element, get_elements, get_all_elements, get_betatron_tune_monitor, get_chromaticity_tuning, get_crm_tuning, get_tune_tuning, get_trm_tuning, get_orbit_tuning, get_orm_tuning, get_dispersion_tuning, now fully superseded by holder[name]/holder.get(name), holder.diagnostic.get(name), and holder.tool.get(name).

Changes to existing functionality

  • ElementHolder.get() signature changed from no-argument to get(name=None), matching ToolHolder.get()/DiagnosticHolder.get(). Calling it with no argument is unchanged.
  • Every internal caller of the removed methods (diagnostic_holder.py, tune.py, tune_response_matrix.py, chromaticity_monitor.py, bba2.py, and 4 self-calls in element_holder.py) was repointed to the typed replacements.
  • Updated the 3 example scripts/notebook cells and ~44 test call sites across 13 test files that used the removed methods.

Testing

The following tests (compatible with pytest) were added:

  • test_get_resolves_a_named_array_regardless_of_its_concrete_family in tests/common/test_element_holder_collection.py, asserting get() resolves a magnet array (HCORR) and a BPM array (BPMS), not just a generic element array — covering the exact gap flagged in review.

Verify that your checklist complies with the project

  • New and existing unit tests pass locally

  • Tests were added to prove that all features/changes are effective

  • The code is commented where appropriate

  • Any existing features are not broken

  • ElementHolder.get() signature changed from no-argument to get(name=None), matching ToolHolder.get()/DiagnosticHolder.get(). Calling it with no argument is unchanged.

  • Every internal caller of the removed methods (diagnostic_holder.py, tune.py, tune_response_matrix.py, chromaticity_monitor.py, bba2.py, and 4 self-calls in element_holder.py) was repointed to the typed replacements.

  • Updated the 3 example scripts/notebook cells and ~44 test call sites across 13 test files that used the removed methods.

Testing

The following tests (compatible with pytest) were added:

  • test_get_resolves_a_named_array_regardless_of_its_concrete_family in tests/common/test_element_holder_collection.py, asserting get() resolves a magnet array (HCORR) and a BPM array (BPMS), not just a generic element
    array — covering the exact gap flagged in review.

Verify that your checklist complies with the project

  • New and existing unit tests pass locally
  • Tests were added to prove that all features/changes are effective
  • The code is commented where appropriate
  • Any existing features are not broken

…d get_element/get_elements/get_all_elements/get_betatron_tune_monitor/get_*_tuning
@GamelinAl

Copy link
Copy Markdown
Member

Looks good to me.

Just why do we need to keep the get_chromaticity_monitor and get_bba methods? They can be accessed in tools no?

@gupichon

gupichon commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

Just why do we need to keep the get_chromaticity_monitor and get_bba methods? They can be accessed in tools no?

Because I missed it.

@gupichon

gupichon commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

@GamelinAl , it's done.

@gupichon
gupichon merged commit 5a9cb5c into 199-elementholder-api-refurbishment Oct 2, 2026
3 checks passed
@gupichon
gupichon deleted the elementholder-api-refurbishment-api-cleaning branch October 2, 2026 07:55
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.

3 participants