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

Declare that an AbstractDimStack is an AbstractArray with namedtuple eltype? #870

Open
tiemvanderdeure opened this issue Nov 27, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@tiemvanderdeure
Copy link
Collaborator

tiemvanderdeure commented Nov 27, 2024

After #811 merged, any DimStack now behaves a lot like an AbstractArray that iterates NamedTuples. Should we declare it as such?

It could even be an AbstractBasicDimArray, but that would require some changes as a AbstractDimStack does not have its dimension in its type, currently.

It would allow us to remove a whole list of array-like methods, and make additional things work, e.g.:

A = [1.0 2.0 3.0; 4.0 5.0 6.0]
dimz = (X([:a, :b]), Y(10.0:10.0:30.0))
da1 = DimArray(A, dimz; name=:one)
da2 = DimArray(Float32.(2A), dimz; name=:two)
da3 = DimArray(Int.(3A), dimz; name=:three)

s = DimStack((da1, da2, da3))

broadcast!(s, s) do x
    (one = 1, two = 2, three = 3)
end

This now fails because ndims is not implemented for DimStack (an oversight, I assume), but if I just add
abstract type AbstractDimStack{K,T,N,L} <: AbstractArray{T,N} end
then it just works.

@rafaqz
Copy link
Owner

rafaqz commented Nov 27, 2024

Yeah we are getting closer. There's an old PR already for this

(There may still be some problems where we break the interface in some places)

@tiemvanderdeure
Copy link
Collaborator Author

I can't find that old PR but I think we're getting really close. It will be a bit of work cleaning up all the method ambiguities, though.

Maybe we could try to see how hard it is to get tests to pass?

@rafaqz
Copy link
Owner

rafaqz commented Nov 27, 2024

Yeah weird I cant either. I'm sure I tried that.

But yes just adding the supertype and running tests would be interesting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants