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.

Sign up for GitHub

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

Get eo bands defined in assets only #243

Merged
merged 6 commits into from
Jan 13, 2021
Merged

Get eo bands defined in assets only #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6552327
Get eo bands from assets only
emmanuelmathot Nov 30, 2020
20b6722
removed trailing whitespace
emmanuelmathot Nov 30, 2020
774af36
key
emmanuelmathot Nov 30, 2020
6baf0d1
only when asset is None
emmanuelmathot Nov 30, 2020
991065f
Added test
emmanuelmathot Nov 30, 2020
3a282e5
removed #
emmanuelmathot Nov 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion pystac/extensions/eo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ def get_bands(self, asset=None):
"""Gets an Item or an Asset bands.

If an Asset is supplied and the bands property exists on the Asset,
returns the Asset's value. Otherwise returns the Item's value
returns the Asset's value. Otherwise returns the Item's value or
all the asset's eo bands

Returns:
List[Band]
Expand All @@ -65,6 +66,13 @@ def get_bands(self, asset=None):
else:
bands = self.item.properties.get('eo:bands')

# get assets with eo:bands even if not in item
if asset is None and bands is None:
bands = []
for (key, value) in self.item.get_assets().items():
if 'eo:bands' in value.properties:
bands.extend(value.properties.get('eo:bands'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


if bands is not None:
bands = [Band(b) for b in bands]

Expand Down
4 changes: 4 additions & 0 deletions tests/extensions/test_eo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ def test_asset_bands(self):
asset_bands = eo_item.ext.eo.get_bands(index_asset)
self.assertIs(None, asset_bands)

# No asset specified
asset_bands = eo_item.ext.eo.get_bands()
self.assertIsNot(None, asset_bands)

# Set
b2_asset = eo_item.assets['B2']
self.assertEqual(eo_item.ext.eo.get_bands(b2_asset)[0].name, "B2")
Expand Down