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

nc_grids is not normal form #26

Closed
mdsumner opened this issue Feb 13, 2019 · 3 comments
Closed

nc_grids is not normal form #26

mdsumner opened this issue Feb 13, 2019 · 3 comments

Comments

@mdsumner
Copy link
Member

@mdsumner mdsumner commented Feb 13, 2019

From @mdsumner on February 13, 2019 7:29

nc_grids should have distinct grids, and nc_vars should list the grid it's in

library(dplyr)
f <- system.file("extdata/argo", package = "tidync") %>% list.files(full.names = TRUE)
tidync::tidync(f)
nc_vars(f) %>% distinct(name)
nc_grids(f) %>% distinct(grid)

Both should have ndims, nc_grids also nvars

  • check this package
  • check stars
  • check tidync

Following on from ropensci/tidync#77 pain, this will help the default variable or grid selection:

  • no NC_CHAR by default
  • pick the most dims
  • always in native order otherwise

Copied from original issue: ropensci/tidync#78

@mdsumner

This comment has been minimized.

Copy link
Member Author

@mdsumner mdsumner commented Feb 13, 2019

This is actually hard because this is used in quite a lot of places.

Can shortcut now with a function like this scheme, and pick off the variables from top to bottom.

nc_axes(f) %>% dplyr::inner_join(nc_dims(f), c("dimension" = "id")) %>% dplyr::inner_join(nc_vars(f), c("variable" = "name")) %>% arrange(type == "NC_CHAR", -ndims)
@mdsumner

This comment has been minimized.

Copy link
Member Author

@mdsumner mdsumner commented Feb 13, 2019

Now trying nesting column "variables" within nc_grids, so the output is normal but we can simply unnest to list every grid with every variable. It also works along with the change in priority-choice of the default variable/grid.

mdsumner added a commit that referenced this issue Feb 14, 2019
@mdsumner

This comment has been minimized.

Copy link
Member Author

@mdsumner mdsumner commented Feb 14, 2019

Looking good. b433604

@mdsumner mdsumner closed this Feb 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.