replace run config .py files with .yaml

[marxide]

Changes the pipeline run configuration file format to YAML.
Implements a strict YAML validator.
Closes #479.

To do:

• Change configuration to YAML
• Update tests
• Rebase on docs-update-adam
• Update docs
• Update CHANGELOG.md

[ajstewart]

@marxide I think the validation errors may need some clarification. Say if an image was missing, previously the error would give you a good clue as to what was wrong, for example, if there is a missing background image, see the error messages below.

With YAML the error seems a little cryptic for someone not familiar with the code, we can tell what's wrong but I don't think the average user will.

old

Pipeline error: Number of BACKGROUND_FILES different from number of IMAGE_FILES files.
Traceback (most recent call last):
  File "/Users/adam/GitHub/vast-pipeline/vast_pipeline/management/commands/runpipeline.py", line 103, in run_pipe
    pipeline.validate_cfg(user=user)
  File "/Users/adam/GitHub/vast-pipeline/vast_pipeline/pipeline/main.py", line 349, in validate_cfg
    'Number of BACKGROUND_FILES different from number of'
vast_pipeline.pipeline.errors.PipelineConfigError: Pipeline error: Number of BACKGROUND_FILES different from number of IMAGE_FILES files.
CommandError: Config error:
Pipeline error: Number of BACKGROUND_FILES different from number of IMAGE_FILES files.

YAML:

Pipeline error: while parsing a mapping
  in "run config", line 93, column 1:
      background:
    ^ (line: 93)
required key(s) '14' not found
  in "run config", line 119, column 1:
        - /Users/adam/askap/pipeline ...
    ^ (line: 119).
Traceback (most recent call last):
  File "/Users/adam/GitHub/vast-pipeline/vast_pipeline/pipeline/config.py", line 262, in validate
    for epoch in epochs
  File "/Users/adam/anaconda3/envs/vast-pipeline-dev/lib/python3.7/site-packages/strictyaml/representation.py", line 110, in revalidate
    result = schema(self._chunk)
  File "/Users/adam/anaconda3/envs/vast-pipeline-dev/lib/python3.7/site-packages/strictyaml/validators.py", line 17, in __call__
    self.validate(chunk)
  File "/Users/adam/anaconda3/envs/vast-pipeline-dev/lib/python3.7/site-packages/strictyaml/compound.py", line 203, in validate
    sorted(list(set(self._required_keys).difference(found_keys)))
  File "/Users/adam/anaconda3/envs/vast-pipeline-dev/lib/python3.7/site-packages/strictyaml/yamllocation.py", line 51, in while_parsing_found
    self.expecting_but_found("while parsing {0}".format(what), found=found)
  File "/Users/adam/anaconda3/envs/vast-pipeline-dev/lib/python3.7/site-packages/strictyaml/yamllocation.py", line 47, in expecting_but_found
    self,
strictyaml.exceptions.YAMLValidationError: while parsing a mapping
  in "run config", line 93, column 1:
      background:
    ^ (line: 93)
required key(s) '14' not found
  in "run config", line 119, column 1:
        - /Users/adam/askap/pipeline ...
    ^ (line: 119)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/adam/GitHub/vast-pipeline/vast_pipeline/management/commands/runpipeline.py", line 105, in run_pipe
    pipeline.config.validate(user=user)
  File "/Users/adam/GitHub/vast-pipeline/vast_pipeline/pipeline/config.py", line 267, in validate
    raise PipelineConfigError(e)
vast_pipeline.pipeline.errors.PipelineConfigError: Pipeline error: while parsing a mapping
  in "run config", line 93, column 1:
      background:
    ^ (line: 93)
required key(s) '14' not found
  in "run config", line 119, column 1:
        - /Users/adam/askap/pipeline ...
    ^ (line: 119).
CommandError: Config error:
Pipeline error: while parsing a mapping
  in "run config", line 93, column 1:
      background:
    ^ (line: 93)
required key(s) '14' not found
  in "run config", line 119, column 1:
        - /Users/adam/askap/pipeline ...
    ^ (line: 119).

[ajstewart]

Very minor points alongside the config validation messages I posted about earlier, I do think they have become too cryptic.

It's very nice to have a defined type of the config, that was causing me a lot of pain in the docstrings!

Thanks for doing it, from testing it locally it all seems to be working as expected through the UI and CLI.

[ajstewart]

This comment might need updating to reflect better what's going on now.

[ajstewart]

@marxide do you think this bit still needs changing to remove references to the input dictionaries etc?

[marxide]

@marxide I think the validation errors may need some clarification.

I agree in that case it is cryptic. I've updated the error messages for that particular validation check and some others in b4b9e60. The standard schema checks will still have the original error messages, but I don't think those ones are very cryptic. Let me know if you come across one that is and we can look at catching it.

[ajstewart]

@marxide I think the validation errors may need some clarification.

I agree in that case it is cryptic. I've updated the error messages for that particular validation check and some others in b4b9e60. The standard schema checks will still have the original error messages, but I don't think those ones are very cryptic. Let me know if you come across one that is and we can look at catching it.

@marxide is there anyway to make the errors even more specific? For example if I remove a background image the error is now (which is of course better!):

Error in config validation:
Pipeline error: Number of epochs per input type do not match..
pipeline_config.validate(user=request.user)
File "/Users/adam/GitHub/vast-pipeline/vast_pipeline/pipeline/config.py", line 261, in validate
raise PipelineConfigError("Number of epochs per input type do not match.")
vast_pipeline.pipeline.errors.PipelineConfigError: Pipeline error: Number of epochs per input type do not match..

But before it would point state clearly that the number of background images does not match the images so the user knows exactly what to fix (note the double full stop there from somewhere, I copied it from the website). It could be an extra bit of info attached to this error message which I think is a good general one.

On a different point, I've noticed that when you create a run through the website without specifying any background files and monitor off the background heading is missing in the config:

  image:
  # list input images here, e.g. (note the leading hyphens)
  # - /path/to/image1.fits
  # - /path/to/image2.fits

  selavy:
  # list input selavy catalogues here, as above with the images

  noise:
  # list input noise (rms) images here, as above with the images

  # Required only if source_monitoring.monitor is true, otherwise optional. If not providing
  # background images, remove the entire background section below.

source_monitoring:

is it possible to make this appear regardless? As if a user went back to add background images they may not know what heading to enter as it's not there. The comment is also referring to a section that isn't present in this example.

[marxide]

Changed the error messages to be more specific. I thought it would be best to explicitly list the epoch or file counts when there is an error. Some examples below.

When there is a background file missing:

Error in config validation
Pipeline error: The number of files must match for all input types.
image has 2 files
selavy has 2 files
noise has 2 files
background has 1 file.

When there is a Selavy file missing for one of the epochs in epoch-mode:

Error in config validation
Pipeline error: The number of files per epoch does not match between input types.
image:
ep1: 3
ep2: 2
ep3: 2
selavy:
ep1: 3
ep2: 1
ep3: 2
noise:
ep1: 3
ep2: 2
ep3: 2.

When one of the Selavy epochs doesn't match the others in epoch-mode:

Error in config validation
Pipeline error: The name of the epochs must match for all input types.
image has 3 epochs: ep1, ep2, ep3
selavy has 3 epochs: ep1, ep2, ep4
noise has 3 epochs: ep1, ep2, ep3.

The background section will now always be printed to the config file when create via the website. It will be commented out when source monitoring is turned off.

[ajstewart]

Thanks for addressing those points, it all looks great!

I think this is now good to go (don't forget to change the merge branch to dev).