Repository: dcs4cop/xcube (python)

Issue:
--proc does not work with plugged in input processors (xcube gen)
**Describe the bug**
When creating a data cube with using xcube gen and specifying the input processor by  the parameter --proc and selecting e.g. "snap-olci-highroc-l2' an error message is thrown. 

`Error: Invalid value for "--proc" / "-p": invalid choice: snap-olci-highroc-l2. (choose from )`

![image](https://user-images.githubusercontent.com/23457217/60656476-c4a0fa80-9e4f-11e9-9d9a-7264bd898f22.png)

This error does not occur when specifying the input processor within a configuration file: 

input_processor: snap-olci-highroc-l2

**To Reproduce**
Steps to reproduce the behavior:
1. Set-up an xcube-gen-* input processor 
2. Use the input processor name with the parameter --proc in the command line interface. 


**Expected behavior**
No error should occur when using --proc for specifying the input processor. 

**Current workaround**
Use configuration file for data cube generation and specify the input processor within the configuration file.


Pull request:
Forman 120 proc option bug
* can use the `xcube gen` CLI with `--proc` option again
* avoid eager imports in CLI modules --> makes all the "xcube COMMAND --help" much faster
* due to no-so-good code smell (reuse same Python variable for different value types):
  * renamed `xcube gen` config param `input_proc` into `input_proc_name` (`input_proc` still works)
  * renamed `xcube gen` config param `output_writer` into `output_writer_name` (`output_writer` still works)

closes #120

Patch:
diff --git a/CHANGES.md b/CHANGES.md
index fd840bd1..223e1638 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -10,6 +10,7 @@
 
 ### Fixes
 
+* It was no longer possible to use the `xcube gen` CLI with `--proc` option. (#120)
 * `totalCount` attribute of time series returned by Web API `ts/{dataset}/{variable}/{geom-type}` now
    contains the correct number of possible observations. Was always `1` before.
 * Renamed Web API function `ts/{dataset}/{variable}/places` into
diff --git a/README.md b/README.md
index 98a2e6ea..5319d638 100644
--- a/README.md
+++ b/README.md
@@ -222,23 +222,21 @@ Generate data cube.
       input paths to be used.
 
     Options:
-      -p, --proc []                   Input processor type name. The choices as
-                                      input processor and additional information
-                                      about input processors  can be accessed by
+      -p, --proc TEXT                 Input processor name. The available input
+                                      processor names and additional information
+                                      about input processors can be accessed by
                                       calling xcube gen --info . Defaults to
-                                      "default" - the default input processor that
-                                      can deal with most common datasets
-                                      conforming with the CF conventions.
+                                      "default", an input processor that can deal
+                                      with simple datasets whose variables have
+                                      dimensions ("lat", "lon") and conform with
+                                      the CF conventions.
       -c, --config TEXT               Data cube configuration file in YAML format.
                                       More than one config input file is
                                       allowed.When passing several config files,
                                       they are merged considering the order passed
                                       via command line.
       -o, --output TEXT               Output path. Defaults to 'out.zarr'
-      -f, --format [csv|mem|netcdf4|zarr]
-                                      Output format. The choices for the output
-                                      format are: ['csv', 'mem', 'netcdf4',
-                                      'zarr']. Additional information about output
+      -f, --format TEXT               Output format. Information about output
                                       formats can be accessed by calling xcube gen
                                       --info. If omitted, the format will be
                                       guessed from the given output path.
@@ -249,14 +247,14 @@ Generate data cube.
       -v, --variables, --vars TEXT    Variables to be included in output. Comma-
                                       separated list of names which may contain
                                       wildcard characters "*" and "?".
-      --resampling [Nearest|Bilinear|Cubic|CubicSpline|Lanczos|Average|Min|Max|Median|Mode|Q1|Q3]
+      --resampling [Average|Bilinear|Cubic|CubicSpline|Lanczos|Max|Median|Min|Mode|Nearest|Q1|Q3]
                                       Fallback spatial resampling algorithm to be
                                       used for all variables. Defaults to
                                       'Nearest'. The choices for the resampling
-                                      algorithm are: dict_keys(['Nearest',
-                                      'Bilinear', 'Cubic', 'CubicSpline',
-                                      'Lanczos', 'Average', 'Min', 'Max',
-                                      'Median', 'Mode', 'Q1', 'Q3'])
+                                      algorithm are: ['Average', 'Bilinear',
+                                      'Cubic', 'CubicSpline', 'Lanczos', 'Max',
+                                      'Median', 'Min', 'Mode', 'Nearest', 'Q1',
+                                      'Q3']
       -a, --append                    Deprecated. The command will now always
                                       create, insert, replace, or append input
                                       slices.
diff --git a/xcube/api/gen/config.py b/xcube/api/gen/config.py
index b9d79d2b..0cf3ca38 100644
--- a/xcube/api/gen/config.py
+++ b/xcube/api/gen/config.py
@@ -18,6 +18,7 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
+
 from typing import Dict, Union
 
 from ...util.config import to_name_dict_pairs, flatten_dict, load_configs
@@ -33,9 +34,9 @@ def get_config_dict(config_obj: Dict[str, Union[str, bool, int, float, list, dic
     """
     config_file = config_obj.get("config_file")
     input_paths = config_obj.get("input_paths")
-    input_processor = config_obj.get("input_processor")
+    input_processor_name = config_obj.get("input_processor_name", config_obj.get("input_processor"))
     output_path = config_obj.get("output_path")
-    output_writer = config_obj.get("output_writer")
+    output_writer_name = config_obj.get("output_writer_name", config_obj.get("output_writer"))
     output_size = config_obj.get("output_size")
     output_region = config_obj.get("output_region")
     output_variables = config_obj.get("output_variables")
@@ -54,14 +55,14 @@ def get_config_dict(config_obj: Dict[str, Union[str, bool, int, float, list, dic
         else:
             config['input_paths'] = input_paths
 
-    if input_processor is not None:
-        config['input_processor'] = input_processor
+    if input_processor_name is not None:
+        config['input_processor_name'] = input_processor_name
 
     if output_path is not None and 'output_path' not in config:
         config['output_path'] = output_path
 
-    if output_writer is not None and 'output_writer' not in config:
-        config['output_writer'] = output_writer
+    if output_writer_name is not None and 'output_writer_name' not in config:
+        config['output_writer_name'] = output_writer_name
 
     if output_resampling is not None and 'output_resampling' not in config:
         config['output_resampling'] = output_resampling
diff --git a/xcube/api/gen/gen.py b/xcube/api/gen/gen.py
index 21597fb6..73d9b11a 100644
--- a/xcube/api/gen/gen.py
+++ b/xcube/api/gen/gen.py
@@ -41,15 +41,15 @@ from ...util.update import update_dataset_attrs, update_dataset_var_attrs, updat
 
 
 def gen_cube(input_paths: Sequence[str] = None,
-             input_processor: str = None,
+             input_processor_name: str = None,
              input_processor_params: Dict = None,
-             input_reader: str = None,
+             input_reader_name: str = None,
              input_reader_params: Dict[str, Any] = None,
              output_region: Tuple[float, float, float, float] = None,
              output_size: Tuple[int, int] = DEFAULT_OUTPUT_SIZE,
              output_resampling: str = DEFAULT_OUTPUT_RESAMPLING,
              output_path: str = DEFAULT_OUTPUT_PATH,
-             output_writer: str = None,
+             output_writer_name: str = None,
              output_writer_params: Dict[str, Any] = None,
              output_metadata: NameAnyDict = None,
              output_variables: NameDictPairList = None,
@@ -64,16 +64,17 @@ def gen_cube(input_paths: Sequence[str] = None,
 
     :param sort_mode:
     :param input_paths: The input paths.
-    :param input_processor: Name of a registered input processor (xcube.api.gen.inputprocessor.InputProcessor)
-           to be used to transform the inputs
+    :param input_processor_name: Name of a registered input processor
+        (xcube.api.gen.inputprocessor.InputProcessor) to be used to transform the inputs.
     :param input_processor_params: Parameters to be passed to the input processor.
-    :param input_reader: Name of a registered input reader (xcube.api.util.dsio.DatasetIO).
+    :param input_reader_name: Name of a registered input reader (xcube.api.util.dsio.DatasetIO).
     :param input_reader_params: Parameters passed to the input reader.
     :param output_region: Output region as tuple of floats: (lon_min, lat_min, lon_max, lat_max).
     :param output_size: The spatial dimensions of the output as tuple of ints: (width, height).
     :param output_resampling: The resampling method for the output.
     :param output_path: The output directory.
-    :param output_writer: Name of an output writer (xcube.api.util.dsio.DatasetIO) used to write the cube.
+    :param output_writer_name: Name of an output writer
+        (xcube.api.util.dsio.DatasetIO) used to write the cube.
     :param output_writer_params: Parameters passed to the output writer.
     :param output_metadata: Extra metadata passed to output cube.
     :param output_variables: Output variables.
@@ -87,16 +88,16 @@ def gen_cube(input_paths: Sequence[str] = None,
     # Force loading of plugins
     __import__('xcube.util.plugin')
 
-    if not input_processor:
-        raise ValueError('Missing input_processor')
-
     if append_mode is not None:
         warnings.warn('append_mode in gen_cube() is deprecated, '
                       'time slices will now always be inserted, replaced, or appended.')
 
-    input_processor = get_input_processor(input_processor)
+    if not input_processor_name:
+        raise ValueError('Missing input_processor_name')
+
+    input_processor = get_input_processor(input_processor_name)
     if not input_processor:
-        raise ValueError(f'Unknown input_processor {input_processor!r}')
+        raise ValueError(f'Unknown input_processor_name {input_processor_name!r}')
 
     if input_processor_params:
         try:
@@ -104,19 +105,19 @@ def gen_cube(input_paths: Sequence[str] = None,
         except TypeError as e:
             raise ValueError(f'Invalid input_processor_params {input_processor_params!r}') from e
 
-    input_reader = find_dataset_io(input_reader or input_processor.input_reader)
+    input_reader = find_dataset_io(input_reader_name or input_processor.input_reader)
     if not input_reader:
-        raise ValueError(f'Unknown input_reader {input_reader!r}')
+        raise ValueError(f'Unknown input_reader_name {input_reader_name!r}')
 
     if not output_path:
         raise ValueError('Missing output_path')
 
-    output_writer = output_writer or guess_dataset_format(output_path)
-    if not output_writer:
-        raise ValueError(f'Failed to guess output_writer from path {output_path}')
-    output_writer = find_dataset_io(output_writer, modes={'w', 'a'})
+    output_writer_name = output_writer_name or guess_dataset_format(output_path)
+    if not output_writer_name:
+        raise ValueError(f'Failed to guess output_writer_name from path {output_path}')
+    output_writer = find_dataset_io(output_writer_name, modes={'w', 'a'})
     if not output_writer:
-        raise ValueError(f'Unknown output_writer {output_writer!r}')
+        raise ValueError(f'Unknown output_writer_name {output_writer_name!r}')
 
     if monitor is None:
         # noinspection PyUnusedLocal
diff --git a/xcube/cli/cli.py b/xcube/cli/cli.py
index 74d82b52..2b99f02b 100644
--- a/xcube/cli/cli.py
+++ b/xcube/cli/cli.py
@@ -3,10 +3,10 @@ import sys
 import click
 
 from xcube.cli.apply import apply
-from xcube.cli.optimize import optimize
 from xcube.cli.extract import extract
 from xcube.cli.gen import gen
 from xcube.cli.grid import grid
+from xcube.cli.optimize import optimize
 from xcube.cli.prune import prune
 from xcube.cli.resample import resample
 from xcube.cli.serve import serve
@@ -78,13 +78,13 @@ DEFAULT_TILE_SIZE = 512
                    'for imaging purposes.')
 @click.option('--tile-size', '-t', metavar='<tile-size>',
               help=f'Tile size, given as single integer number or as <tile-width>,<tile-height>. '
-              f'If omitted, the tile size will be derived from the <input>\'s '
-              f'internal spatial chunk sizes. '
-              f'If the <input> is not chunked, tile size will be {DEFAULT_TILE_SIZE}.')
+                   f'If omitted, the tile size will be derived from the <input>\'s '
+                   f'internal spatial chunk sizes. '
+                   f'If the <input> is not chunked, tile size will be {DEFAULT_TILE_SIZE}.')
 @click.option('--num-levels-max', '-n', metavar='<num-levels-max>', type=int,
               help=f'Maximum number of levels to generate. '
-              f'If not given, the number of levels will be derived from '
-              f'spatial dimension and tile sizes.')
+                   f'If not given, the number of levels will be derived from '
+                   f'spatial dimension and tile sizes.')
 def level(input, output, link, tile_size, num_levels_max):
     """
     Generate multi-resolution levels.
diff --git a/xcube/cli/gen.py b/xcube/cli/gen.py
index 586e8694..299ebbbf 100644
--- a/xcube/cli/gen.py
+++ b/xcube/cli/gen.py
@@ -21,37 +21,29 @@
 
 import click
 
-from xcube.api.gen.config import get_config_dict
 from xcube.api.gen.defaults import DEFAULT_OUTPUT_PATH, DEFAULT_OUTPUT_RESAMPLING
-from xcube.api.gen.gen import gen_cube
-from xcube.api.gen.iproc import InputProcessor
-from xcube.util.dsio import query_dataset_io
-from xcube.util.objreg import get_obj_registry
-from xcube.util.reproject import NAME_TO_GDAL_RESAMPLE_ALG
+from xcube.util.constants import RESAMPLING_METHOD_NAMES
 
-input_processor_names = [input_processor.name
-                         for input_processor in get_obj_registry().get_all(type=InputProcessor)]
-output_writer_names = [ds_io.name for ds_io in query_dataset_io(lambda ds_io: 'w' in ds_io.modes)]
-resampling_algs = NAME_TO_GDAL_RESAMPLE_ALG.keys()
+resampling_methods = sorted(RESAMPLING_METHOD_NAMES)
 
 
 # noinspection PyShadowingBuiltins
 @click.command(name='gen', context_settings={"ignore_unknown_options": True})
 @click.argument('inputs', nargs=-1)
-@click.option('--proc', '-p', type=click.Choice(input_processor_names),
-              help=f'Input processor type name. '
-                   f'The choices as input processor and additional information about input processors '
-                   ' can be accessed by calling xcube gen --info . Defaults to "default" - the default input processor '
-                   'that can deal with most common datasets conforming with the CF conventions.')
+@click.option('--proc', '-p', default='default',
+              help=f'Input processor name. '
+                   f'The available input processor names and additional information about input processors '
+                   'can be accessed by calling xcube gen --info . Defaults to "default", an input processor '
+                   'that can deal with simple datasets whose variables have dimensions ("lat", "lon") and '
+                   'conform with the CF conventions.')
 @click.option('--config', '-c', multiple=True,
               help='Data cube configuration file in YAML format. More than one config input file is allowed.'
                    'When passing several config files, they are merged considering the order passed via command line.')
 @click.option('--output', '-o', default=DEFAULT_OUTPUT_PATH,
               help=f'Output path. Defaults to {DEFAULT_OUTPUT_PATH!r}')
-@click.option('--format', '-f', type=click.Choice(output_writer_names),
+@click.option('--format', '-f',
               help=f'Output format. '
-                   f'The choices for the output format are: {output_writer_names}.'
-                   ' Additional information about output formats can be accessed by calling '
+                   'Information about output formats can be accessed by calling '
                    'xcube gen --info. If omitted, the format will be guessed from the given output path.')
 @click.option('--size', '-s',
               help='Output size in pixels using format "<width>,<height>".')
@@ -60,11 +52,11 @@ resampling_algs = NAME_TO_GDAL_RESAMPLE_ALG.keys()
 @click.option('--variables', '--vars', '-v',
               help='Variables to be included in output. '
                    'Comma-separated list of names which may contain wildcard characters "*" and "?".')
-@click.option('--resampling', type=click.Choice(resampling_algs),
+@click.option('--resampling', type=click.Choice(resampling_methods),
               default=DEFAULT_OUTPUT_RESAMPLING,
               help='Fallback spatial resampling algorithm to be used for all variables. '
                    f'Defaults to {DEFAULT_OUTPUT_RESAMPLING!r}. '
-                   f'The choices for the resampling algorithm are: {resampling_algs}')
+                   f'The choices for the resampling algorithm are: {resampling_methods}')
 @click.option('--append', '-a', is_flag=True,
               help='Deprecated. The command will now always create, insert, replace, or append input slices.')
 @click.option('--prof', is_flag=True,
@@ -98,10 +90,10 @@ def gen(inputs: str,
     ".txt" extension which contains the actual input paths to be used.
     """
     input_paths = inputs
-    input_processor = proc
+    input_processor_name = proc
     config_file = config
     output_path = output
-    output_writer = format
+    output_writer_name = format
     output_size = size
     output_region = region
     output_variables = variables
@@ -111,6 +103,8 @@ def gen(inputs: str,
     info_mode = info
     sort_mode = sort
 
+    from xcube.api.gen.config import get_config_dict
+    from xcube.api.gen.gen import gen_cube
     # Force loading of plugins
     __import__('xcube.util.plugin')
 
@@ -128,7 +122,10 @@ def gen(inputs: str,
 
 
 def _format_info():
-    # noinspection PyUnresolvedReferences
+    from xcube.api.gen.iproc import InputProcessor
+    from xcube.util.dsio import query_dataset_io
+    from xcube.util.objreg import get_obj_registry
+
     input_processors = get_obj_registry().get_all(type=InputProcessor)
     output_writers = query_dataset_io(lambda ds_io: 'w' in ds_io.modes)
 
diff --git a/xcube/cli/prune.py b/xcube/cli/prune.py
index 234de30a..b0469c6e 100644
--- a/xcube/cli/prune.py
+++ b/xcube/cli/prune.py
@@ -24,7 +24,6 @@ import os.path
 
 import click
 
-from xcube.util.chunk import get_empty_dataset_chunks
 from xcube.util.constants import FORMAT_NAME_ZARR
 
 
@@ -47,6 +46,7 @@ def _prune(input_path: str = None,
            dry_run: bool = False,
            monitor=None):
     from xcube.api import open_cube
+    from xcube.util.chunk import get_empty_dataset_chunks
     from xcube.util.dsio import guess_dataset_format
 
     input_format = guess_dataset_format(input_path)
diff --git a/xcube/util/constants.py b/xcube/util/constants.py
index 9d33b3fe..1f0f62b2 100644
--- a/xcube/util/constants.py
+++ b/xcube/util/constants.py
@@ -40,4 +40,23 @@ GEOGCS["WGS 84",
 """
 FORMAT_NAME_ZARR = "zarr"
 FORMAT_NAME_NETCDF4 = "netcdf4"
-FORMAT_NAME_MEM = "mem"
\ No newline at end of file
+FORMAT_NAME_MEM = "mem"
+
+# Note: this list must be kept in-sync with xcube/util/reproject.py:NAME_TO_GDAL_RESAMPLE_ALG
+RESAMPLING_METHOD_NAMES = {
+    # Up-sampling
+    'Nearest',
+    'Bilinear',
+    'Cubic',
+    'CubicSpline',
+    'Lanczos',
+
+    # Down-sampling
+    'Average',
+    'Min',
+    'Max',
+    'Median',
+    'Mode',
+    'Q1',
+    'Q3',
+}


Test patch:
diff --git a/test/api/gen/default/test_gen.py b/test/api/gen/default/test_gen.py
index 9d436050..0f15122f 100644
--- a/test/api/gen/default/test_gen.py
+++ b/test/api/gen/default/test_gen.py
@@ -92,9 +92,23 @@ class DefaultProcessTest(unittest.TestCase):
         self.assertIn('lon', ds.coords)
         self.assertFalse(np.any(ds.coords['lon'] > 180.))
 
+    def test_illegal_proc(self):
+        with self.assertRaises(ValueError) as e:
+            gen_cube_wrapper(
+                [get_inputdata_path('20170101120000-UKMO-L4_GHRSST-SSTfnd-OSTIAanom-GLOB-v02.0-fv02.0.nc')],
+                'l2c-single.zarr', sort_mode=True, input_processor_name=None)
+        self.assertEqual('Missing input_processor_name', f'{e.exception}')
+
+        with self.assertRaises(ValueError) as e:
+            gen_cube_wrapper(
+                [get_inputdata_path('20170101120000-UKMO-L4_GHRSST-SSTfnd-OSTIAanom-GLOB-v02.0-fv02.0.nc')],
+                'l2c-single.zarr', sort_mode=True, input_processor_name='chris-proba')
+        self.assertEqual("Unknown input_processor_name 'chris-proba'", f'{e.exception}')
+
 
 # noinspection PyShadowingBuiltins
-def gen_cube_wrapper(input_paths, output_path, sort_mode=False) -> Tuple[bool, Optional[str]]:
+def gen_cube_wrapper(input_paths, output_path, sort_mode=False, input_processor_name='default') \
+        -> Tuple[bool, Optional[str]]:
     output = None
 
     def output_monitor(msg):
@@ -105,7 +119,7 @@ def gen_cube_wrapper(input_paths, output_path, sort_mode=False) -> Tuple[bool, O
             output += msg + '\n'
 
     config = get_config_dict(dict(input_paths=input_paths, output_path=output_path))
-    return gen_cube(input_processor='default',
+    return gen_cube(input_processor_name=input_processor_name,
                     output_size=(320, 180),
                     output_region=(-4., 47., 12., 56.),
                     output_resampling='Nearest',
diff --git a/test/cli/test_gen.py b/test/cli/test_gen.py
index f11f40c4..c40d6495 100644
--- a/test/cli/test_gen.py
+++ b/test/cli/test_gen.py
@@ -15,10 +15,14 @@ class GenCliTest(CliTest):
         result = self.invoke_cli(['gen', '--info'])
         self.assertEqual(0, result.exit_code)
 
-    def test_missing_args(self):
+    def test_default_proc_is_valid(self):
+        result = self.invoke_cli(['gen', '-p', 'default', 'input.nc'])
+        self.assertEqual(0, result.exit_code)
+
+    def test_main_with_illegal_proc(self):
         with self.assertRaises(ValueError) as cm:
-            self.invoke_cli(['gen'])
-        self.assertEqual("Missing input_processor", f'{cm.exception}')
+            self.invoke_cli(['gen', '--proc', 'gnartz'])
+        self.assertEqual("Unknown input_processor_name 'gnartz'", f'{cm.exception}')
 
     def test_main_with_illegal_size_option(self):
         with self.assertRaises(ValueError) as cm:
@@ -46,8 +50,4 @@ class GenCliTest(CliTest):
             self.invoke_cli(['gen', '-c', 'nonono.yml', 'input.nc'])
         self.assertEqual("Cannot find configuration 'nonono.yml'", f'{cm.exception}')
 
-    def test_main_with_illegal_options(self):
-        with self.assertRaises(ValueError) as cm:
-            self.invoke_cli(['gen', 'input.nc'])
-        self.assertEqual('Missing input_processor', f'{cm.exception}')