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

Move to using getproperty for accessing RasterStack layers? #231

Closed
rafaqz opened this issue Nov 20, 2021 · 3 comments
Closed

Move to using getproperty for accessing RasterStack layers? #231

rafaqz opened this issue Nov 20, 2021 · 3 comments

Comments

@rafaqz
Copy link
Owner

rafaqz commented Nov 20, 2021

It would be nice if we could use Symbol and String to to access categorical variables in DimensionalData.jl and inheriting packages, without wrapping them with At. But there are some edge cases where that clashes with DimStack/RasterStack getindex syntax, and will effect users of Rasters.jl the most.

rafaqz/DimensionalData.jl#338 (comment)

The problem generally is that DimStack/RasterStack overloads the Array/NamedTuple combination, in ways that are sometimes confusing, but also pretty powerful. Currently we use getindex of a single Symbol to retrieve a stack layer, and any other getindex to retrieve the values of all of the Raster layers, or slice them all.

I'm considering migrating the syntax to getproperty, like this:

rasterstack[:layername]
# To
rasterstack.layername

Then rasterstack[args...] is always indexing the values of the layers, and never fetching a layer (after a period of depreciation)

There are probably a few more changes like this that need to happen, because as @mkborregaard has said, RasterStack getindex is pretty weird - first(rasterstack) returns a single Raster layer while rasterstack[1] returns a NamedTuple of the first value of all the layers !!!. This is because iteration happens over whole layers, like a NamedTuple of Array, but for getindex a stack is is like an Array of NamedTuple.

We might need to contain that weirdness somehow. One option is to not use getindex for retrieving values at all, which resolves the problem. But then you can't do:

rasterstack[X=Between(100, 160), Y=Between(0, 40)]

Which is one of the main things I do with a RasterStack after loading a netcdf. If anyone has any thoughts or feedback, that would be great, I don't want to break everyones workflow unless it makes sense.

@vlandau @mauro3 @mkborregaard @jamesmaino @virgile-baudrot

@rafaqz rafaqz changed the title Move to using getproperty for accessing RasterStack layers Move to using getproperty for accessing RasterStack layers? Nov 20, 2021
@jamesmaino
Copy link

I'm trying hard to remember back to when we first discussed this weirdness. I think I was advocating hyperdimensional arrays where time is an index but there were issues with that setup too. We never thought of using the getproperty syntax. Definitely a big change though...

What would be the alternative for rasterstack[X=Between(100, 160), Y=Between(0, 40)]?

@rafaqz
Copy link
Owner Author

rafaqz commented Nov 22, 2021

I never thought about it before either, I just realised the other day that its actually the main way to index a NamedTuple, and that's pretty much what a stack is. (I think we were discussing just mushing all of the indexing into one flat getindex even for RasterSeries... just to make things even weirder. But I'm kind of glad we didn't... slice/combine make that kind of redundant now, and they are quite ordinary in comparison.)

I don't know what an alternative to that getindex example could be, I really like it as-is honestly, and that its exactly the same as for an array. Others seem to as well, @maxfreu wanted to use RasterStack specifically because its so easy to index multiple layers at once.

Maybe we could use another function name for indexing stacks, but the [ ] brackets are hard to beat for style.

@rafaqz
Copy link
Owner Author

rafaqz commented Mar 2, 2023

Completed

@rafaqz rafaqz closed this as completed Mar 2, 2023
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

No branches or pull requests

2 participants