Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support extracting sub-axis #132

Closed
wants to merge 2 commits into from
Closed

Conversation

bgctw
Copy link

@bgctw bgctw commented May 30, 2022

general approach to extract several subcompoennts.

For #129 one can do: cv[Axis(:a,:b)]

@jonniedie
Copy link
Owner

Hey, thanks for the PR! I actually already have multi-symbol indexing in the newest update of ComponentArrays, though. I decided to go in a different direction than this PR for the following reasons:

  1. Indexing into an array with another array means something specific already. So having something like cv1[cv2] should mean "Index into cv1 at indices defined by the values of cv2" For example:
julia> cv = ComponentVector(a=(a1=100,a2=(a21=210, a22=220)), b=2, c = (c1=reshape(1:4,(2,2)),));

julia> cv[[1, 2, 4]]
3-element Vector{Int64}:
 100
 210
   2

julia> cv[ComponentArray(x=2:3, y=6)]
ComponentVector{Int64}(x = [210, 220], y = 2)

Changing this behavior would break from the expected interface of AbstractArrays and may break some people's code that depends on this behavior.

  1. Axis(:a, :b) is a shorthand constructor for Axis(a=1, b=2), which means "An Axis where the named a component gets index 1 and b gets index 2". So this would be a valid axis for a ComponentVector like this:
ComponentVector(a=200, b=42)

but not this:

ComponentVector(a=200, b=[42, 13])

The correct axis type for the last example would look like

Axis(a=1, b=2:3)

@bgctw
Copy link
Author

bgctw commented May 31, 2022

Thanks for the explanation.
I agree that indexing with another ComponentArray should be avoided.

However, the indexing by an Axis could be nice and quite general.

Can the current indexing support my use case #133?

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