caracal-pipeline / stimela

Stimela 2.0
GNU General Public License v2.0
5 stars 3 forks source link

Difficulty passing multiple tuple arguments via stimela cab interface #327

Open talonmyburgh opened 1 month ago

talonmyburgh commented 1 month ago

The script I wrote for Dynamic Spectra Normalisation and Smoothing requires passing one or more smoothing kernel sizes to the script. Below is an extract from inspect_dynspec.py which shows the @click interface I used to set this up:

def parse_tuples(ctx, param, value: str) -> list:
    # Parse each value (which is expected to be a string) into a tuple
    parsed_tuples = []
    for val in value:
        # Assuming the input is in the format "(1,2)" without spaces
        # Strip parentheses and split by comma
        stripped_val = val[1:-1]  # Remove the parentheses
        tuple_vals = tuple(map(int, stripped_val.split(",")))
        parsed_tuples.append(tuple_vals)
    return parsed_tuples
.
.
.
@click.option(
    "--kernel",
    callback=parse_tuples,
    multiple=True,
    help="Kernel sizes as tuples: (nu_delta (MHz), t_delta (s)) i.e. python inspect_dynspec.py --kernel '(2,3)' --kernel '(3,4)'",
)

This allowed for kernels to be passed as python inspect_dynspec.py <blah> --kernel '(3,4)' --kernel '(10,20)'...

To get similar behaviour via the stimela wrapper I wrote this cab. However input:

  kernel:
    info: "Kernel sizes as tuples: (nu_delta (MHz), t_delta (s)) i.e. python inspect_dynspec.py --kernel '(2,3)' --kernel '(3,4)'"
    dtype: Tuple[float, float]
    required: false
    default: (1,1)
    policies: 
      format_list: "({0},{1})"

does not work and gives error:

$ stimela doc stimela/inspect_dynspec.yml

2024-08-05 11:31:59 STIMELA INFO: starting                                                                                                                                                              
2024-08-05 11:31:59 STIMELA INFO: loaded full configuration from cache                                                                                                                                  
2024-08-05 11:32:00 STIMELA INFO: saving config dependencies to ./stimela.config.deps                                                                                                                   
2024-08-05 11:32:00 STIMELA INFO: will load recipe/config file stimela/inspect_dynspec.yml                                                                                                              
2024-08-05 11:32:00 STIMELA INFO: loaded 1 cab definition(s) and 0 recipe(s)                                                                                                                            
Traceback (most recent call last):
  File "/home/myburgh/venv/stimela_venv/bin/stimela", line 6, in <module>
    sys.exit(cli())
  File "/home/myburgh/venv/stimela_venv/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/home/myburgh/venv/stimela_venv/lib/python3.10/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/home/myburgh/venv/stimela_venv/lib/python3.10/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/myburgh/venv/stimela_venv/lib/python3.10/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/myburgh/venv/stimela_venv/lib/python3.10/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File "/home/myburgh/repos/stimela/stimela/commands/doc.py", line 125, in doc
    cab = Cab(**stimela.CONFIG.cabs[name])
  File "<string>", line 19, in __init__
  File "/home/myburgh/repos/stimela/stimela/kitchen/cab.py", line 114, in __post_init__
    Cargo.__post_init__(self)
  File "/home/myburgh/repos/stimela/scabha/cargo.py", line 320, in __post_init__
    self.inputs = Cargo.flatten_schemas(OrderedDict(), self.inputs, "inputs")
  File "/home/myburgh/repos/stimela/scabha/cargo.py", line 297, in flatten_schemas
    raise SchemaError(f"{label}.{name} is not a valid parameter definition", exc0) from None
scabha.exceptions.SchemaError: inputs.kernel is not a valid parameter definition: Invalid value assigned: AnyNode is not a ListConfig, list or tuple.
    full_key: policies.format_list
    reference_type=ParameterPolicies
    object_type=ParameterPolicies

I changed bracketing in the default key from default: (1,1) to default: [1,1] but this made no difference.

This is using the latest stimela version available on master branch though I found the same issue on venv-activation that was merged into master a little while back.

o-smirnov commented 1 month ago

Ah I see the problem. Try format_list: ["({0},{1})"] instead. This is actually documented, but I fear I led you astray with my earlier advice.

I'm a little puzzled/bothered that the error message looks so ugly for you. When I tried a reproducer, I got a much neater error, which immediately made the problem apparent:

image

o-smirnov commented 1 month ago

Ah ok I get the "ugly" error when I try to doc your cab myself. So let's keep this issue open until I fix the error message.

talonmyburgh commented 1 month ago

Missed that in the documentation, apologies. That is substantially prettier - not sure why I don't get such niceties :(

Perhaps this requires a new issue (I see there is issue #219 that perhaps points to it), but my script accepts a path for output products to be saved to. If the folder does not exist it is created. However, parameter validation by stimela complains about the path not existing and I don't see any policy or parameter option to stop validation checks. Should I omit this folder creation step in my script and let stimela do it (if it does)?

image

talonmyburgh commented 1 month ago

Oh wait I see parameter must_exist: false ... trying it now.

That worked :D

o-smirnov commented 1 month ago

There's must_exist and mkdir... but I think you're better off specifying the output in the outputs section, then the default behaviours are mote sensible.

talonmyburgh commented 1 month ago

Will try that route, thanks.