nsidc / snow-today-webapp-server

Serve data the snow-today-webapp depends on
https://snow-today-webapp-server.readthedocs.io/
MIT License
4 stars 0 forks source link

Update region hierarchy to explicitly pattern regions and collections #29

Closed MattF-NSIDC closed 11 months ago

MattF-NSIDC commented 11 months ago

Collections have regions have collections have regions, etc. etc.

The reason for this change is without making them explicit, we don't have a way to differentiate between them. Regions and collections are both just string keys that could have dictionary values.

I felt null was a good way to represent regions without any children instead of {}. It's twice as many characters, but it's easier to spot at a glance. Not a strong preference, we can always go back.

For the lowest level of the hierarchy, e.g. huc8, it would make sense to be able to specific a list instead of a dictionary of regions that are in that level, since those regions would never have children. Perhaps we can make the schema permit both.

MattF-NSIDC commented 11 months ago

pre-commit.ci autofix

sebastien-lenard commented 11 months ago

Regions have ids, collections haven't right now, just codes. But i can add the collections level.

{} vs null. I may prefer keeping {}, which simplifies the handling code.

The last level may have children at some point. Actually we could imagine that some branches go deeper than others. In terms of code, again, i would prefer to have a similar pattern whatever the level of type items

sebastien-lenard commented 11 months ago

@MattF-NSIDC 1/ I tried to read some json by matlab and think i made a mistake by putting the dictionary keys as int rather than strings. Matlab can't read integer keys. Is that ok for you if I shift back to string keys?

2/ I tried the null rather than empty {} and matlab can read the json, but there's some complexities after that to handle the tree in a recursive way (the way I handle it for writing right now).

3/ It'll be probably the same for the list rather than a dictionnary.

Although supercomputer backend in matlab won't probably ever read these json files, it might be safer to have a really uniform syntax in the format of the json file for the supercomputer backend developer who will take in charge this code in the future, or for debugging on our side.

Don't hesitate to tell me your thoughts :)

MattF-NSIDC commented 11 months ago

1/ I tried to read some json by matlab and think i made a mistake by putting the dictionary keys as int rather than strings. Matlab can't read integer keys. Is that ok for you if I shift back to string keys?

In fact, all keys must be strings under the JSON spec! I already ran prettier and it automatically converted the keys from integers to strings.

2/ I tried the null rather than empty {} and matlab can read the json, but there's some complexities after that to handle the tree in a recursive way (the way I handle it for writing right now).

3/ It'll be probably the same for the list rather than a dictionnary.

No problem, I'm happy to go with {} for a region with no children :) Having the structure be 100% consistently made of dictionaries sounds fine too; we can think about convenience things later on if we feel the need.