Using dictionary format under [meta.concat] fails

[hillierdani]

Thanks for sharing and maintaining wellmap! I use it extensively but now face a new issue:

Documentation specifies:
"The paths can be specified either as a string, a list, or a dictionary. "

I have this in my TOML file:

[meta.concat]
TRX-0032 = ['../human/TRX-0032_dmem_20240327.toml', '../human/TRX-0032_20231004.toml']
TRX-0033 = ['../human/TRX-0033_dmem_20240327.toml', '../human/TRX-0033_20231004.toml']

Call to wellmap.load on this aggregator TOML file fails at

  File "/Users/me/.pyenv/versions/3.11.6/lib/python3.11/pathlib.py", line 493, in _parse_args
    a = os.fspath(a)
        ^^^^^^^^^^^^
TypeError: expected str, bytes or os.PathLike object, not dict

Did I miss here something or the documentation is not in line with the actual implementation?
Thanks for any hints on how to get this feature working!

[hillierdani]

Diving into the code I guess the documentation can be misunderstood, I had a wishful thinking that a dict of lists is also valid. It is not right now, here my mod to file.py from line 440:

        elif isinstance(paths, dict):
            paths1 = []
            for pk, pv in paths.items():
                if isinstance(pv, list):
                    paths1.extend([(pk, x) for x in pv])
                elif isinstance(pv, str):
                    paths1.append((pk, pv))
            paths = paths1

This seems to work though wellmap.show produces somewhat weird results but the data itself is parsed correctly and works well in my downstream analysis.
Happy to produce a pull request - though I do not think my workaround is the best way to extend the functionality here, a recursive solution would be more elegant.

[kalekundert]

Yeah, sorry that the documentation wasn't clear, but you're right that dicts-of-lists are not currently supported.

If you want to make a PR, that would be great. Don't bother trying to make a more general recursive algorithm, though. I think that would undesirable for two reasons. First is that I want to reserve the dict-of-dicts and list-of-dicts forms for specifying more complicated concatenation options, should there ever be a need for any, similar to how [meta.include] works. Second is that the whole point of a dictionary is to provide a name for each concatenated layout. Splitting this name over multiple levels of dictionaries would be a pretty marginal benefit, and it'd also introduce the question of how the various dictionary keys should be concatenated (e.g. include a dot between the keys or not?). I think it's best to avoid all that and only allow one level of keys.

This may go without saying, but be sure to include some tests as well. I imagine that the tests for this feature would belong in test_config_from_toml.nt. The existing test cases with id: concat-* should be good references. Let me know if you have any questions; the tests are not that easy to understand. The following links may be helpful:

• Parametrize From File
• NestedText

[hillierdani]

Thanks very much for pointing me towards the test system. I learn here (using pytest extensively myself but not at the magician level as you do). However I got stuck at actually running the tests. I could add a new case in test_config_from_toml.nt but pyproject.toml is not using poetry. While I am pretty happy with poetry (and pycharm), I can imagine reasons you prefer to keep things as they are. If so, please point me (or future contributors to wellmap) to some hints on how to run/add tests. Thank you.

[kalekundert]

You should just need to run:

$ cd wellmap/tests
$ pytest

If you aren't used to using pytest from the command line, here's a command that only runs tests from the file test_config_from_toml.py that contain the keyword concat. This can be more convenient during development:

$ pytest test_config_from_toml.py -k concat

If either of these commands aren't working, can you tell me what you tried and what the output you're getting?