Supported integer suffix for step headings in the user config file - config_reader

[sverhoeven]

You are about to submit a new Pull Request. Before continuing make sure you read the contributing guidelines and you comply with the following criteria:

• You have stick to Python. Talk with us before adding other programming languages to HADDOCK3
• Your PR is about CNS
• Your code is well documented: proper docstrings and explanatory comments for those tricky parts
• You structured the code into small functions as much as possible. You can use classes if there's a (state) purpose
• code follows our coding style
• You wrote tests for the new code
• tox tests pass. Run tox command inside the repository folder
• -test.cfg examples execute without errors. Inside examples/ run python run_tests.py -b
• PR does not add any install dependencies unless permission granted by the HADDOCK team
• PR does not break licensing
• Your PR is about writing documentation for already existing code 🔥
• Your PR is about writing tests for already existing code

---

In https://github.com/i-VRESSE/workflow-builder I use a toml library to write the config file. The toml library does not support repeats of the same header. To repeat headers I need to make them unique by appending an index and putting quotes around the module name.

This PR adds support to haddock3 to parse config file like:

[headerone]
val = 10

['headerone.2']
val = 20

['headerone.3']
val = 30

to

{
  "headerone": {'val': 10},
  "headerone.1": {'val': 20},
  "headerone.2": {'val': 30},
}

This config file can be written or loaded by any standard compliant toml library and should make it easier for haddock3 config files to be produced or consumed by other programs.

[codecov-commenter]

Codecov Report

Merging #473 (b9d74a2) into main (dcb1b6f) will increase coverage by 0.12%.
The diff coverage is 97.29%.

@@            Coverage Diff             @@
##             main     #473      +/-   ##
==========================================
+ Coverage   74.66%   74.78%   +0.12%     
==========================================
  Files         105      105              
  Lines        6926     6963      +37     
==========================================
+ Hits         5171     5207      +36     
- Misses       1755     1756       +1     

Impacted Files	Coverage Δ
src/haddock/gear/config_reader.py	95.55% <93.33%> (-0.21%)	⬇️
tests/test_config_reader.py	95.93% <100.00%> (+0.44%)	⬆️
tests/test_gear_config_writer.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[joaomcteixeira]

Hi @sverhoeven

Good addition!

The only caveat I see on this is the difference between the two config styles for users. But I perfectly understand the restraints required by the web interface. It's just a matter that in the official examples we advertise only the one without integers. FYI, we have already a config writer module in case it helps you.

Some technical aspect I am trying to sort out is that our config requires the multiline lists to have an empty line afterwards. I think I need to sort that out before this PR can go in because toml does not have such a requirement.

Another thing, you also need to operate in this function:

haddock3/src/haddock/gear/config_reader.py

Line 378 in 7d4bb34

def _update_key_number(key, d, sep='.', offset=0):

You need to key.split(".")[0] before the function applies the index. You can add that inside the function itself. I would rather not skip the function in case there is a mix of "with index" and "without index" in the config file. In the later case, the index should be ignored and haddock3 should handle it internally (by the _update_key_number function).

And, I think this is all. 😉

p.s. - an alternative would that the web interface create a toml string (internally) and then quickly remove the header integers before saving the string to the file. I don't know if this is possible in the web interface. But, it's an idea. With the python json module, for example, it is possible to generate the string before saving it to the file.

Cheers,

[sverhoeven]

The test_read_config test has been expanded to test mix of "with index" and "without index".
Added a short description in the module doc string that headers with index is a thing.

haddock3 and the workflow-builder both need to read and write config files.
it would be nice that both can read "with index" or "without index" and haddock3 only writes "without index" and the workflow-builder only writes "with index" .

[joaomcteixeira]

Okay, I will accommodate that. We can have:

1. reading with and without index automatically parsed by the function
2. writing with or without index by boolean flag.

Sounds good? ☺️

Internally, the dictionaries entering the writing function have indexes (by obviously reasons). In writing, indexes are removed here:

haddock3/src/haddock/gear/config_writer.py

Line 106 in 7d4bb34

module_key = get_module_name(key)

for example:

>>> get_module_name("topoaa.1")
"topoaa"

[rvhonorato]

Thanks for the contribution @sverhoeven, having haddock3 support toml out of the box will be very helpful for future integrations!

[rvhonorato]

it would be nice that both can read "with index" or "without index" and haddock3 only writes "without index" and the workflow-builder only writes "with index" .

@joaomcteixeira are you working on this or should I try?

[joaomcteixeira]

You can work on it.

[joaomcteixeira]

I have now reviewed this pull request. Sorry for the long delay, as it was still "draft" I thought it was still a work in progress. It seems very okay to merge. Thanks!

[joaomcteixeira]

it would be nice that both can read "with index" or "without index" and haddock3 only writes "without index" and the workflow-builder only writes "with index" .

I would leave this for another Pull Request following #518

We can work with toml reading edited strings instead of files directly, i suggest.

[joaomcteixeira]

lint issues above are addressed here: #523

[sverhoeven]

Thanks for fixing the linting errors, I have merged main and now CI and tox -e lint are green.

[joaomcteixeira]

Hi @sverhoeven

a quick question before merging. Why do the headers need to be quoted to have the indexes? Is it because to avoid conflicts with parameters that are dictionaries? If so, I think we can safely edit the regexes because no parameters start with digits. Let me know, Cheers.

[sverhoeven]

Hi @sverhoeven

a quick question before merging. Why do the headers need to be quoted to have the indexes? Is it because to avoid conflicts with parameters that are dictionaries? If so, I think we can safely edit the regexes because no parameters start with digits. Let me know, Cheers.

TOML libraries treat . in table name with and without quotes differently.

For example

[foo]
x = 1

['foo.1']
x = 2

[bar]
y = 1

[bar.1]
y = 2

will be parsed as

{
    "foo": {"x": 1}, 
    "foo.1": {"x": 2}, 
    "bar": {
        "y": 1, 
        "1": {"y": 2}
    }
 }

Having [bar] and [bar.1] will give keys on different levels, while with quotes the keys are on same level.
So using quotes gives a data structure, more like what the parser does with repeated table names (without digits or qoutes).

[joaomcteixeira]

closed by #539
Thanks!