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

Allow BasicDimArrays like DimPoints, DimIndices, etc in DimStack #928

Closed
wants to merge 1 commit into from

Conversation

asinghvi17
Copy link
Collaborator

this allows:

DimStack((; data = da, points = DimPoints(da)))`

very useful if you are trying to calculate e.g slope with stencils, since you then have the coordinates available to you.

@asinghvi17 asinghvi17 self-assigned this Feb 17, 2025
@asinghvi17 asinghvi17 added the enhancement New feature or request label Feb 17, 2025
@rafaqz
Copy link
Owner

rafaqz commented Feb 17, 2025

Hmmm. It doesn't really fit the model of what a DimStack is.

There is a rule across DD of no duplication of dimensions. That keeps everything very simple and sane as it can be

We may need a ProtoDimindices <: AbstractArray etc that has gets turned into DimIndices, like an Array becomes a DimArray when combined with dimensions. (Yes this happens totally at compile time for every getindex)

Then if you set new dimensions, DimPoints etc layers would also be updated, etc etc

@rafaqz
Copy link
Owner

rafaqz commented Feb 17, 2025

Or Proto{DimIndices}(args) I think could work for everything lazy.

@asinghvi17
Copy link
Collaborator Author

I can see where you're going with that, maybe that makes sense? But it's very confusing to tell the user that DimPoints is not a DimArray. We don't necessarily need to promote this workflow but it should either work or error informatively...

@rafaqz
Copy link
Owner

rafaqz commented Feb 17, 2025

I mean, it literally cant work the way DimStack works. DimPoints is not a DimArray.

An AbstractDimArray wraps another array, it even has a type parameter A for the type of that other array - thats the fundamental split with AbstractDimArray and AbstractBasicDimArray.

You can of course just collect DimIndices etc and use them as an array. But its a very different thing. The values are no longer actually connected to the dimensions.

@rafaqz
Copy link
Owner

rafaqz commented Feb 27, 2025

This approach isn't really workable, so closing

@rafaqz rafaqz closed this Feb 27, 2025
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

Successfully merging this pull request may close these issues.

2 participants