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

Band names should be properties instead of object keys #142

Closed
ghost opened this issue Aug 1, 2018 · 7 comments
Closed

Band names should be properties instead of object keys #142

ghost opened this issue Aug 1, 2018 · 7 comments
Milestone

Comments

@ghost
Copy link

ghost commented Aug 1, 2018

The band definitions use a string as the key for the bands object. The names of those keys don't seem well defined. The examples show "B1" and "B01".

In order to search for all items with a nir band, it would be helpful to be able to index the band definitions. As it is now, the band names would need to be consistent across all items to be easily searchable. For example "eo:bands.common_name=nir" is easier than "eo:bands.B1.common_name=nir" where B1 is unknown ahead of time or mixed with B01 across items.

I would suggest bands be changed to an array and band_name be added to the band definition instead of as a key

@hgs-msmith

@ghost ghost added the stac-sprint-3-discuss label Aug 1, 2018
@m-mohr
Copy link
Collaborator

m-mohr commented Aug 2, 2018

This sounds related to the discussion of #127 (comment). With the band names as keys it is easier to access them directly by band name and to make sure they are unique. Of course that is a problem in the searching scenario and maybe just a application-specific optimization as mentioned in the referenced comment.

@ghost
Copy link
Author

ghost commented Aug 2, 2018

Are band names well known? Most of the time band names seem to be hinting at band order, which I think is well defined. Arrays are a better fit for representing order as object keys are not ordered in JSON. I guess I'm unsure when I would use a string to reference "band 3" instead of band[2].

Also, if you know the band name ahead of time, don't you already know the band information? Band definitions generally don't change very often.

@jeffnaus
Copy link
Contributor

The EO spec includes band name in its extension writeup, here: https://github.com/radiantearth/stac-spec/blob/master/extensions/stac-eo-spec.md

The comon_name element of the band object is the value.

We probably should make a more complete list of common band names.

@cholmes cholmes added this to the 0.6.0 milestone Aug 21, 2018
@ghost
Copy link
Author

ghost commented Aug 30, 2018

I wanted to add an additional data point here.

My team was just looking as adding a dataset with a red edge band and it ended up looking like:

eo:bands: {
...
   'red edge': {
      common_name: 'red edge',
      center_wavelength: 0.710,
      full_width_half_max: 0.040
    }
}

I usually try pretty hard to avoid spaces in object keys. We could certainly provide a rule like underscore replacement, but that is adding unneeded complexity.

Also, the common_name already provides that information.

For the HSI case, the band order is what matters and that is lost in the dictionary lookup. Are we also going to add a band index field that has the order? In my opinion, that is what an array is for.

@m-mohr
Copy link
Collaborator

m-mohr commented Aug 31, 2018

I always thought the common name had to be one of the listed ones. I certainly agree that there are common names missing, so they should be added. And in case only names from common names are allowed, we could simply define the names and replace spaces with underscores. That would also increase interoperability as things that are not defined will eventually get written differently, e.g. RED EDGE, red_edge, red-edge, etc.

Generally, I think the pros and cons here are very similar to the one we are discussing for assets in #198, #151 and others. I see valid reasons for both objects and arrays. I try to not think to much about implementation details as these usually can be solved by different implementations, it should be more important to think about the content itself. And for bands (in contrast to assets) I certainly see one major reason to it being arrays: The bands usually have some kind of order and this should be reflected by being an array. One drawback seems to be that it doesn't seem so intuitive to reference the bands by band name, e.g. in the assets, as it is not intuitively clear which property is used for referencing (name? common_name? ...)

@ghost
Copy link
Author

ghost commented Aug 31, 2018

I think asset names have some value, so I'm happy to let that conversation play out. But I don't think it is possible to have a complete list of band names. We work with hyperspectral data with 200+ bands and names are not meaningful in that case. We can certainly generate a display/common name, but it is shouldn't be used as part of the structure.

@ghost
Copy link
Author

ghost commented Sep 21, 2018

#247 addressed this issue

@ghost ghost closed this as completed Sep 21, 2018
This issue was closed.
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

3 participants