Unnecessary list of one element per iteration in loop in YAML checker?

Open
#477 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
35/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Stale
Tech stack
python
Domain
tooling

Research direction

Start in httomo/yaml_checker.py at lines 172-195 and inspect how conf and template_yaml_conf are constructed and related. Confirm whether they always correspond one-to-one, then simplify the lookup and remove yml_method_list if safe while preserving the parameter compatibility check.

Written by the indexing model from the issue text.

Description

question refactor

Specifically, this is about the check for that parameters in the config for a method are in the template for that method: https://github.com/DiamondLightSource/httomo/blob/7842bae27c1777545279aa36e8c10d9c4eeb489f/httomo/yaml_checker.py#L172-L195

For some basic info:

  • conf is a list of method configs parsed from the YAML pipeline file to a python list
  • template_yaml_conf is a list of templates associated with the methods in the pipeline, parsed to python list

It seems like in the loop over the method configs in the pipeline file, the YAML template config associated with that method is searched for, and put in the the yml_method_list variable, which is a list: https://github.com/DiamondLightSource/httomo/blob/7842bae27c1777545279aa36e8c10d9c4eeb489f/httomo/yaml_checker.py#L177-L183

This is a bit confusing, since I would have expected that the conf and template_yaml_conf lists are the same length, due to each method config having one and only one YAML template associated with it. There doesn't seem to be a need to "search" for the YAML template config associated with each method, I would have thought doing an iteration over both conf and template_yaml_conf something would accomplish this, like:

for method_config, template_config in zip(conf, template_yaml_config):
  # TODO: check params in `method_config` are compatible with `template_config`, maybe using sets or something

and there's maybe no need for yml_method_list (again, which is a list only ever containing one element)?

Dominant language
Python
Stars
10
Forks
5
Avg merge
5d 17h
Merged PRs (30d)
1

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from DiamondLightSource/httomo

All issues in DiamondLightSource/httomo

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.