GeoscienceAustralia / ga_sar_workflow

InSAR processing workflow used by Geoscience Australia
Apache License 2.0
3 stars 1 forks source link

Update cleanup parameter type #166

Closed sixy6e closed 3 years ago

sixy6e commented 3 years ago

The cleanup parameter used in several of our Tasks such as:

https://github.com/GeoscienceAustralia/gamma_insar/blob/pygamma_workflow/insar/scripts/process_gamma.py#L199

doesn't provide the greatest clarity especially when configuring via a config file. The variable then gets treated almost like it is a boolean, when in fact it is not. The current config specifies:

cleanup = true

However, the parameter type will simply be defined as a string. Users wanting the opposite behaviour, are likely to then specify:

cleanup = false

However, the type will still be a string, and continue with a cleanup.

I propose we change the type to a luigi.BoolParameter (see here).

The default behaviour is false, and at the command line, one can use --cleanup and it'll pass through as True. Via a config, it can be specified as above, and luigi will convert the type to a Python bool, eg:

cleanup = true
mwheeler commented 3 years ago

Closing this issue, this has been resolved for quite a while (I think it was fixed sometime in 2020), tasks w/ the cleanup parameter now declare it as: cleanup = luigi.BoolParameter()

eg: https://github.com/GeoscienceAustralia/gamma_insar/blob/pygamma_workflow/insar/scripts/process_gamma.py#L1351-L1353