Repository: dcs4cop/xcube-sh

Issue:
Cannot omit band names for "CUSTOM" datasets
Currently, when opening a cube given a `CubeConfig` with `dataset_name="CUSTOM"` and given `collection_id`, then users must also pass `band_names` otherwise an error occurs:

    SentinelHubError: 500 Server Error: Internal Server Error for url: https://creodias.sentinel-hub.com/api/v1/process/dataset/{collection_id}/bands

This is not user-friendly, because users need to know the available band names (and their native sample types) in advance.

Discussion:

Patch:
diff --git a/xcube_sh/chunkstore.py b/xcube_sh/chunkstore.py
index d0f3125..b26c497 100644
--- a/xcube_sh/chunkstore.py
+++ b/xcube_sh/chunkstore.py
@@ -553,12 +553,16 @@ class SentinelHubChunkStore(RemoteStore):
                  trace_store_calls=False):
         self._sentinel_hub = sentinel_hub
         if cube_config.band_names is None:
-            d = cube_config.to_dict()
-            d['band_names'] = sentinel_hub.band_names(
-                cube_config.dataset_name
-                if cube_config.dataset_name != 'CUSTOM'
-                else cube_config.collection_id
+            bands = sentinel_hub.bands(
+                cube_config.dataset_name,
+                collection_id=cube_config.collection_id
             )
+            band_names = [b['name'] for b in bands]
+            band_sample_types = [b.get('sampleType') for b in bands]
+            d = cube_config.to_dict()
+            d['band_names'] = band_names
+            if all(band_sample_types):
+                d['band_sample_types'] = band_sample_types
             cube_config = CubeConfig.from_dict(d)
         super().__init__(cube_config,
                          observer=observer,
@@ -626,7 +630,7 @@ class SentinelHubChunkStore(RemoteStore):
                 self.cube_config.dataset_name,
                 band_name, default='FLOAT32'
             )
-        elif isinstance(band_sample_types, tuple):
+        elif isinstance(band_sample_types, (tuple, list)):
             index = self.cube_config.band_names.index(band_name)
             sample_type = band_sample_types[index]
         else:  # isinstance(band_sample_types, str)
diff --git a/xcube_sh/config.py b/xcube_sh/config.py
index 48c8527..c6e0ea1 100644
--- a/xcube_sh/config.py
+++ b/xcube_sh/config.py
@@ -30,13 +30,13 @@ from xcube.util.assertions import assert_given, assert_true, assert_in
 from .constants import CRS_ID_TO_URI
 from .constants import CRS_URI_TO_ID
 from .constants import DEFAULT_CRS
+from .constants import DEFAULT_MOSAICKING_ORDER
+from .constants import DEFAULT_RESAMPLING
 from .constants import DEFAULT_TILE_SIZE
 from .constants import DEFAULT_TIME_TOLERANCE
-from .constants import SH_MAX_IMAGE_SIZE
-from .constants import RESAMPLINGS
-from .constants import DEFAULT_RESAMPLING
-from .constants import DEFAULT_MOSAICKING_ORDER
 from .constants import MOSAICKING_ORDERS
+from .constants import RESAMPLINGS
+from .constants import SH_MAX_IMAGE_SIZE
 
 
 def _safe_int_div(x: int, y: int) -> int:
@@ -335,11 +335,11 @@ class CubeConfig:
         return self._band_names
 
     @property
-    def band_units(self) -> Union[None, str, Tuple[str, ...]]:
+    def band_units(self) -> Union[None, str, Sequence[str]]:
         return self._band_units
 
     @property
-    def band_sample_types(self) -> Union[None, str, Tuple[str, ...]]:
+    def band_sample_types(self) -> Union[None, str, Sequence[str]]:
         return self._band_sample_types
 
     @property
diff --git a/xcube_sh/constants.py b/xcube_sh/constants.py
index b80b878..597d4c6 100644
--- a/xcube_sh/constants.py
+++ b/xcube_sh/constants.py
@@ -26,6 +26,8 @@ DEFAULT_CLIENT_SECRET = os.environ.get('SH_CLIENT_SECRET')
 
 DEFAULT_SH_API_URL = 'https://services.sentinel-hub.com'
 DEFAULT_SH_OAUTH2_URL = f'{DEFAULT_SH_API_URL}/oauth'
+DEFAULT_SH_METADATA_API_URL = f'{DEFAULT_SH_API_URL}/configuration/' \
+                              f'api/v1/metadata/collection/byoc-%s'
 
 SH_CATALOG_FEATURE_LIMIT = 100  # SH Catalog only allows this number of features to requested.
 
diff --git a/xcube_sh/main.py b/xcube_sh/main.py
index b932c18..8c89371 100644
--- a/xcube_sh/main.py
+++ b/xcube_sh/main.py
@@ -264,11 +264,21 @@ def info(datasets: List[str] = None):
     else:
         response = dict()
         for dataset_name in datasets:
-            band_names = sentinel_hub.band_names(dataset_name)
-            bands = dict()
-            for band_name in band_names:
-                bands[band_name] = sentinel_hub.METADATA.dataset_band(dataset_name, band_name, default={})
-            response[dataset_name] = bands
+            bands = sentinel_hub.bands(dataset_name)
+            bands_dict = dict()
+            for band in bands:
+                band_dict = dict(band)
+                band_name = band_dict.pop('name')
+                band_dict.update(
+                    sentinel_hub.METADATA.dataset_band(dataset_name,
+                                                       band_name,
+                                                       default={})
+                )
+                band_sample_type = band.pop('sampleType', None)
+                if band_sample_type:
+                    band_dict['sample_type'] = band_sample_type
+                bands_dict[band_name] = band_dict
+            response[dataset_name] = bands_dict
     print(json.dumps(response, indent=2))
 
 
diff --git a/xcube_sh/sentinelhub.py b/xcube_sh/sentinelhub.py
index 89307f2..f4f6bfb 100644
--- a/xcube_sh/sentinelhub.py
+++ b/xcube_sh/sentinelhub.py
@@ -43,6 +43,7 @@ from .constants import DEFAULT_RESAMPLING
 from .constants import DEFAULT_RETRY_BACKOFF_BASE
 from .constants import DEFAULT_RETRY_BACKOFF_MAX
 from .constants import DEFAULT_SH_API_URL
+from .constants import DEFAULT_SH_METADATA_API_URL
 from .constants import DEFAULT_SH_OAUTH2_URL
 from .constants import SH_CATALOG_FEATURE_LIMIT
 from .metadata import SentinelHubMetadata
@@ -170,13 +171,33 @@ class SentinelHub:
         SentinelHubError.maybe_raise_for_response(response)
         return response.json()
 
-    def band_names(self, dataset_name: str) -> List[str]:
-        response = self.session.get(self.api_url
-                                    + f'/api/v1/process/dataset/'
-                                      f'{dataset_name}/bands')
+    def band_names(self, dataset_name: str, collection_id: str = None) -> List[str]:
+        if dataset_name.upper() == 'CUSTOM':
+            url = DEFAULT_SH_METADATA_API_URL % collection_id
+            response = self.session.get(url)
+            SentinelHubError.maybe_raise_for_response(response)
+            bands = response.json().get('bands', [])
+            return [band.get('name') for band in bands]
+
+        url = f'{self.api_url}/api/v1/process/dataset/{dataset_name}/bands'
+        response = self.session.get(url)
         SentinelHubError.maybe_raise_for_response(response)
         return response.json().get('data', {})
 
+    def bands(self, dataset_name: str, collection_id: str = None) \
+            -> List[Dict[str, Any]]:
+        if dataset_name.upper() == 'CUSTOM':
+            url = DEFAULT_SH_METADATA_API_URL % collection_id
+            response = self.session.get(url)
+            SentinelHubError.maybe_raise_for_response(response)
+            return response.json().get('bands', [])
+
+        url = f'{self.api_url}/api/v1/process/dataset/{dataset_name}/bands'
+        response = self.session.get(url)
+        SentinelHubError.maybe_raise_for_response(response)
+        band_names = response.json().get('data', [])
+        return [dict(name=band_name) for band_name in band_names]
+
     def collections(self) -> List[Dict[str, Any]]:
         """
         See https://docs.sentinel-hub.com/api/latest/reference/#operation/getCollections
diff --git a/xcube_sh/store.py b/xcube_sh/store.py
index 1ee0732..35ab388 100644
--- a/xcube_sh/store.py
+++ b/xcube_sh/store.py
@@ -294,25 +294,30 @@ class SentinelHubDataOpener(DataOpener):
     def _describe_data(self, data_id: str) -> DatasetDescriptor:
         dataset_metadata, collection_metadata = \
             self._get_dataset_and_collection_metadata(data_id)
-        band_metadatas = dataset_metadata.get('bands', {})
+        bands = dict(dataset_metadata.get('bands', {}))
 
         if self._sentinel_hub is not None and data_id != 'CUSTOM':
             # If we are connected to the API, we return band names by API
-            band_names = self._sentinel_hub.band_names(data_id)
-        else:
-            # Otherwise all we know about
-            band_names = band_metadatas.keys()
+            remote_bands = self._sentinel_hub.bands(data_id)
+            if remote_bands:
+                for band in remote_bands:
+                    band_copy = dict(band)
+                    band_name = band_copy.pop('name')
+                    if 'sampleType' in band_copy:
+                        band_copy['sample_type'] = band_copy.pop('sampleType')
+                    if band_name in bands:
+                        bands[band_name].update(band_copy)
+                    else:
+                        bands[band_name] = band_copy
 
         data_vars = {}
-        for band_name in band_names:
-            band_metadata = band_metadatas.get(band_name,
-                                               dict(sample_type='FLOAT32'))
+        for band_name, band_attrs in bands.items():
             data_vars[band_name] = \
                 VariableDescriptor(
                     name=band_name,
-                    dtype=band_metadata.get('sample_type', 'FLOAT32'),
-                    dims=('time', 'lat', 'lon'),
-                    attrs=band_metadata.copy()
+                    dtype=band_attrs.get('sample_type', 'FLOAT32'),
+                    dims=('time', 'y', 'x'),
+                    attrs=band_attrs.copy()
                 )
 
         dataset_attrs = dataset_metadata.copy()
@@ -323,6 +328,11 @@ class SentinelHubDataOpener(DataOpener):
             extent = collection_metadata.get('extent')
             if extent is not None:
                 bbox = extent.get("spatial", {}).get('bbox')
+                if isinstance(bbox, list) and len(bbox) > 0:
+                    if isinstance(bbox[0], list) and len(bbox[0]) == 4:
+                        bbox = tuple(bbox[0])
+                    elif len(bbox) == 4:
+                        bbox = tuple(bbox)
                 interval = extent.get("temporal", {}).get('interval')
                 if isinstance(interval, list) and len(interval) > 0:
                     if isinstance(interval[0], list) and len(interval[0]) == 2:
@@ -356,11 +366,12 @@ class SentinelHubDataOpener(DataOpener):
 
     def _get_dataset_and_collection_metadata(self, data_id: str) \
             -> Tuple[Dict[str, Any], Optional[Dict[str, Any]]]:
-        if data_id == 'CUSTOM':
+        if data_id.upper() == 'CUSTOM':
             return {}, None
         dataset_metadata = SentinelHubMetadata().datasets.get(data_id)
         if dataset_metadata is None:
-            raise DataStoreError(f'Dataset "{data_id}" not found.')
+            dataset_metadata = {}
+            # raise DataStoreError(f'No metadata for dataset "{data_id}" found.')
         if self._sentinel_hub is not None:
             # If we are connected to the API,
             # we may also have collection metadata


Test patch:
diff --git a/test/test_chunkstore.py b/test/test_chunkstore.py
index 15e94b9..9559bfc 100644
--- a/test/test_chunkstore.py
+++ b/test/test_chunkstore.py
@@ -382,9 +382,12 @@ class SentinelHubMock:
         self._config = config
         self._requests = []
 
-    def band_names(self, dataset_name: str):
+    def band_names(self, dataset_name: str, collection_id=None):
         return S2_BAND_NAMES
 
+    def bands(self, dataset_name: str, collection_id=None):
+        return [dict(name=b) for b in S2_BAND_NAMES]
+
     # noinspection PyUnusedLocal
     def get_features(self, collection_name, bbox, crs, time_range):
         """Return dummy catalog features"""
diff --git a/test/test_config.py b/test/test_config.py
index e022d92..773d218 100644
--- a/test/test_config.py
+++ b/test/test_config.py
@@ -32,10 +32,10 @@ class CubeConfigTest(unittest.TestCase):
     def test_adjust_sizes(self):
         spatial_res = 0.00018
         common_kwargs = dict(dataset_name='S2L2A',
-                             band_names=('B01', 'B02', 'B03'),
+                             band_names=['B01', 'B02', 'B03'],
                              spatial_res=spatial_res,
                              tile_size=(512, 512),
-                             time_range=('2019-01-01', '2019-01-02'))
+                             time_range=['2019-01-01', '2019-01-02'])
 
         # size will be smaller than chunk sizes
         config = CubeConfig(bbox=(10.11, 54.17, 10.14, 54.19), **common_kwargs)
@@ -181,14 +181,19 @@ class CubeConfigTest(unittest.TestCase):
         self.assertEqual("Found invalid parameters in cube configuration: 'geometrix', 'special_res'",
                          f'{cm.exception}')
 
-    def test_band_names_may_be_null(self):
-        config = CubeConfig(dataset_name='S2L2A',
-                            band_names=None,
-                            bbox=(10.11, 54.17, 10.14, 54.19),
-                            spatial_res=0.00001,
-                            time_range=('2019-01-01', '2019-01-02'))
+    def test_band_names(self):
+        common_kwargs = dict(dataset_name='S2L2A',
+                             bbox=(10.11, 54.17, 10.14, 54.19),
+                             spatial_res=0.00001,
+                             time_range=('2019-01-01', '2019-01-02'))
+
+        config = CubeConfig(**common_kwargs)
         self.assertEqual(None, config.band_names)
 
+        config = CubeConfig(band_names=["B02", "B03"], **common_kwargs)
+        self.assertIsInstance(config.band_names, tuple)
+        self.assertEqual(("B02", "B03"), config.band_names)
+
     def test_deprecated_geometry_still_works(self):
         config = CubeConfig(dataset_name='S2L2A',
                             geometry=(10.11, 54.17, 10.14, 54.19),
diff --git a/test/test_sentinelhub.py b/test/test_sentinelhub.py
index 7c72f8b..1e1eb3f 100644
--- a/test/test_sentinelhub.py
+++ b/test/test_sentinelhub.py
@@ -315,7 +315,7 @@ class SentinelHubCatalogueTest(unittest.TestCase):
         self.assertEqual(expected_dataset_names,
                          sentinel_hub.dataset_names)
 
-    def test_variable_names(self):
+    def test_band_names(self):
         expected_band_names = ['B01',
                                'B02',
                                'B03',
@@ -343,6 +343,35 @@ class SentinelHubCatalogueTest(unittest.TestCase):
         self.assertEqual(expected_band_names, sentinel_hub.band_names('S2L2A'))
         sentinel_hub.close()
 
+    def test_bands(self):
+        expected_band_names = ['B01',
+                               'B02',
+                               'B03',
+                               'B04',
+                               'B05',
+                               'B06',
+                               'B07',
+                               'B08',
+                               'B8A',
+                               'B09',
+                               'B10',
+                               'B11',
+                               'B12',
+                               'viewZenithMean',
+                               'viewAzimuthMean',
+                               'sunZenithAngles',
+                               'sunAzimuthAngles']
+        sentinel_hub = SentinelHub(session=SessionMock({
+            'get': {
+                'https://services.sentinel-hub.com/api/v1/process/dataset/S2L2A/bands': {
+                    'data': expected_band_names
+                }
+            }
+        }))
+        self.assertEqual([dict(name=b) for b in expected_band_names],
+                         sentinel_hub.bands('S2L2A'))
+        sentinel_hub.close()
+
     def test_get_features(self):
         properties = [{'datetime': '2019-10-02T10:35:47Z'}, {'datetime': '2019-10-04T10:25:47Z'},
                       {'datetime': '2019-10-05T10:45:36Z'}, {'datetime': '2019-10-05T10:45:44Z'}]