search

huizezhang-sherry / cubble

A tidy structure for spatio-temporal vector data
https://huizezhang-sherry.github.io/cubble/
Other
55 stars 9 forks source link

as_cubble method for stars and make_spatial_sf suggestions #23

Closed loreabad6 closed 7 months ago

loreabad6 commented 7 months ago

Hi @huizezhang-sherry, I was playing a bit with the as_cubble() function to coerce stars objects to cubble and noted a couple of things.

  1. To coerce from a vector data cube the sfc dimension had to be the first dimension in the data cube. This is because of an if check, so I think this could be safely replaced with a check over all dimensions and testing the condition as if any of the dimensions inherits sfc.

  2. The raster data cube coercion assumes that time is the third dimension, but this is not always the case. In the example of the documentation, the band dimension is mistankenly taken as the time dimension. This creates the cubble correctly but trying to call face_temporal() for example, fails since it cannot find the time column in the data. I would think that letting the user define the time dimension would be cleaner and more straightforward. Maybe the examples need to be rethought since it does not make sense to add band as time, but it is theoretically possible to do so!

  3. Also, I was testing the make_spatial_sf() function and noticed that it fails if the lat/long columns are x/y, so I added a line to include that case as well.

huizezhang-sherry commented 7 months ago

Hi @loreabad6,

1/2: thanks, I added additional tests for the cases you mentioned.

3: I realize the fail is because stopifnot assumes long&lat are the coordinates and checks for that, but later codes only use coords() to extract the coordinates, so a better check is to make sure that coords is not null.

Thanks for the contribution!