Skip to content

feat: add generic ElementHolder collection access - #376

Open
gupichon wants to merge 12 commits into
mainfrom
366-feature-add-generic-elementholder-collection-access
Open

feat: add generic ElementHolder collection access#376
gupichon wants to merge 12 commits into
mainfrom
366-feature-add-generic-elementholder-collection-access

Conversation

@gupichon

@gupichon gupichon commented Sep 7, 2026

Copy link
Copy Markdown
Member
  • Return an element or None for exact-name lookups
  • Support typed wildcard selections, slices and insertion-order indexing
  • Add get() and [:] access to all registered elements
  • Reuse array type inference for selections
  • Document the API with NumPy docstrings and readable test examples

- Return an element or None for exact-name lookups
- Support typed wildcard selections, slices and insertion-order indexing
- Add get() and [:] access to all registered elements
- Reuse array type inference for selections
- Document the API with NumPy docstrings and readable test examples
@gupichon gupichon self-assigned this Sep 7, 2026
@gupichon gupichon linked an issue Sep 7, 2026 that may be closed by this pull request
2 tasks
@GamelinAl

Copy link
Copy Markdown
Member

I tried to look a bit a the code a did some tests. Here is what I found:

  1. The __repr__ needs to be fixed, right now it does not output the name and output the full lattice

Not linked to this MR, but it makes things difficult.

main: Simulator(name='design', lattice='sr/lattices/ebs.mat', mat_key=None, linker=None, description=None)
now:  Simulator(ring=Lattice([...thousands of elements...]), description=None, lattice='...', mat_key=None)
  1. Looking up a wrong name gives None istead of an error:
quad = sr.design["blabla"]
quad?
>>> Type:        NoneType
String form: None
Docstring:   <no docstring>
  1. get methods () return list an not arrays:
    For example:
type(sr.design.get_all_elements())
>>> list

The same is true for other get_XXXs methods.
But maybe this needs to go now because we can use sr.design[:], I would think it's better to have one way to do this.

  1. magnets[:] and magnets["QD"] works but return ElementArray instead of MagnetArray
type(sr.design.magnets[":"])
>>> pyaml.arrays.element_array.ElementArray
  1. Similar bug with using subset of magnet arrays
magnets.get()        -> MagnetArray     ✅
magnets.get()[:]     -> ElementArray    ❌ should be MagnetArray
magnets.get()['S*']  -> ElementArray    ❌ should be MagnetArray
  1. The ElementHolder.get method has changed and take no argument
cell1 = sr.design.get("Cell1")
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[24], line 1
----> 1 cell1 = sr.design.get("Cell1")

TypeError: ElementHolder.get() takes 1 positional argument but 2 were given

So I don't know how to access an user defined array


Probably some of the tests should use mixed type arrays to catch this type of bugs.

@JeanLucPons

Copy link
Copy Markdown
Member

Point 1 2 3 6 are normal, it is a work in progress not already done as mentioned in the initial issue.
4 and 5 are clear bugs (or there are unexpeted elements in arrays)...

@gupichon

gupichon commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

I'm not sure why the documentation pipeline is failing. I suspect it's related to the release build on Read the Docs.

@gupichon

gupichon commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Point 1 2 3 6 are normal, it is a work in progress not already done as mentioned in the initial issue. 4 and 5 are clear bugs (or there are unexpeted elements in arrays)...

Yes, I will open a separate issue for the element array type inference.

- Return an element or None for exact-name lookups
- Support typed wildcard selections, slices and insertion-order indexing
- Add get() and [:] access to all registered elements
- Reuse array type inference for selections
- Document the API with NumPy docstrings and readable test examples
…older-collection-access' into 366-feature-add-generic-elementholder-collection-access
@gupichon

gupichon commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Point 1 2 3 6 are normal, it is a work in progress not already done as mentioned in the initial issue. 4 and 5 are clear bugs (or there are unexpeted elements in arrays)...

Regarding point 2: should we raise an error if an unknown value is passed for direct access?
When using an fnmatch pattern, an empty array is returned. To stay consistent, I chose to return None for direct access, but I can raise an exception instead if you prefer. @JeanLucPons @GamelinAl

@gupichon

gupichon commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

4. magnets[:] and magnets["Q_D_"] works but return ElementArray instead of MagnetArray

type(sr.design.magnets[":"])
>>> pyaml.arrays.element_array.ElementArray

Because of the quotes here, you're looking for an element named ":", which results in an empty, generic array. The correct syntax is type(sr.design.magnets[:]), but as @JeanLucPons mentioned, this doesn't work yet.

@GamelinAl

Copy link
Copy Markdown
Member

Point 1 2 3 6 are normal, it is a work in progress not already done as mentioned in the initial issue. 4 and 5 are clear bugs (or there are unexpeted elements in arrays)...

Regarding point 2: should we raise an error if an unknown value is passed for direct access? When using an fnmatch pattern, an empty array is returned. To stay consistent, I chose to return None for direct access, but I can raise an exception instead if you prefer. @JeanLucPons @GamelinAl

I would say it's better to raise an error.

@gupichon

gupichon commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

I would say it's better to raise an error.

Ok, I would like to do this in a future issue to also update ElementArray accordingly.

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.

Feature: Add generic ElementHolder collection access

4 participants