Get eo bands defined in assets only #243
Conversation
|
I join an STAC item example for validation |
Codecov Report
@@ Coverage Diff @@
## develop #243 +/- ##
===========================================
+ Coverage 93.93% 93.97% +0.03%
===========================================
Files 32 32
Lines 3960 3965 +5
===========================================
+ Hits 3720 3726 +6
+ Misses 240 239 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This brings up some general questions for me - is the general rule that, for a property that could be on an item (that is an array) that instead has values across assets, should the item always check asset properties? Or is this particular to eo:bands?
Despite the general rule not being clear, the practical instance of this - where if you ask an Item what it's eo:bands are, and those are defined in the assets, you'd want the right answer - seems like a good addition, especially if you've hit on a use case needing it.
I put a comment around a case where assets could have the same band information - we shouldn't repeat band data in that case. Besides handling and testing that case, this looks good to me!
| bands = [] | ||
| for (key, value) in self.item.get_assets().items(): | ||
| if 'eo:bands' in value.properties: | ||
| bands.extend(value.properties.get('eo:bands')) |
There was a problem hiding this comment.
What happens if multiple assets have the same bands? This could be the case if you have for example RGB and RGBIR imagery in the same Item.
There was a problem hiding this comment.
I see your point. IMO, the eo:bands property should be limited to assets because there are strongly linked. Furthermore, the results of get_bands should be a dictionary key/value with key as the asset key and value the list of eo:bands. My proposal here is to resolve a case that is allowed by the specification.
I would keep it that way even if leading to 'duplicates' in the function returned list. It simply returns what is specified in the item. If I extend your example, the red band of the RGB could be different of the RGBIR because their composition could be made differently (e.g. different center_wavelength based on optical calibration) and thus that would be 2 different bands.
There was a problem hiding this comment.
It's a good point about the bands being potentially different but with the same name - band.name isn't unique according to the spec, so really you'd have to check all properties in order to de-duplicate. I do think this is probably a less common case though.
@matthewhanson you've worked with the eo extension a bunch, do you have an opinion here? This is for a pystac.Item returning bands from the extension call item.ext.eo.get_bands(), returning either the bands specified in the Item or a aggregation of bands specified in the assets - the potential for duplication happening in the latter case.
There was a problem hiding this comment.
In lieu of further input and to move this along, I'm in favor of your suggestion to keep it as is, and we can modify if this seems like the incorrect choice down the road.
Context
As defined in the EO extension specification, the
eo:bandsproperties are allowed in assets without being declared at the item levelIssue
The
get_bandsmethod inEOItemExtdoes not return bands defined in asset level if noassetparameter in specified.Proposed Solution
The
get_bandsmethod inEOItemExtreturn aggregated list of bands defined in asset level if noassetparameter in specified.PR Checklist: