Composable auspice config

[jameshadfield]

Closes #298

See commit messages for details, but there's a few parts I want to call out explicitly:

Merging approach

Specifically, merging of lists is done by extending the original list, rather than simply replacing it. We use a custom approach to decide whether new elements should be added to the list or replace already-present elements. As an example:

{
  "colorings": [
    {"key": "num_date", "title": "date", "type": "continuous"}
  ]
}

{
  "colorings": [
    {"key": "country", "title": "Country", "type": "categorical"},
    {"key": "num_date", "title": "Sampling Date", "type": "continuous"}
  ]
}

Here the second JSON's num_date coloring replaces the first JSONs one, and the country coloring is added in, so we end up with:

{
  "colorings": [
    {"key": "num_date", "title": "Sampling Date", "type": "continuous"}
    {"key": "country", "title": "Country", "type": "categorical"},
  ]
}

I think this is the only practical way to combine colorings in multiple configs, but we also use lists for many other options, e.g. 'panels', 'maintainers'. This PR still uses the same approach, but I'm less convinced for non-coloring options such as these whether we should extend lists or simply replace them.

Note that we don't really do deep merging here at all. This only applies in one situation as far as I can tell, config.display_defaults.panels: list[str] where overlay configs will completely replace the original list, and this is different from how config.panels currently behaves.

Refactoring of warnings code

This changed all usages of (export v2's) warn() to warning(), which in practice only changes the stacklevel, which changes... nothing? The warning() helper function was added years ago but saw ~no usage, so I think we should either use it consistently (as done here) or remove it and just use the default warn().

Validate then merge

The approach here is to validate each config JSON independently, rather than merging then validating. (This works well since our schema has no required properties.) augur validate auspice-config-v2 hasn't been updated to allow multiple auspice configs, but we could do so if we want (requires a little refactoring to avoid circular imports). We could additionally validate the merged config if people want to, although the current implementation guarantees that valid configs produce a valid merged config.

Long term direction

This work paves the way to merging in all config-related command line args within auspice_config.py which should help clean up export_v2.py quite a bit.

I didn't add an option to write out the merged config, but I know this is a subject that's been discussed quite a bit in the context of snakemake config merging so we could add this here if wanted.

Still to do:

• regenerate docs. I need to follow Jover's instructions here as with my setup (py 3.10) I introduce changes to every docs file.
• changelog

[codecov]

Codecov Report

Attention: Patch coverage is 54.20561% with 98 lines in your changes missing coverage. Please review.

Project coverage is 73.24%. Comparing base (8d575c3) to head (f8caa5c).
Report is 165 commits behind head on master.

Files with missing lines	Patch %	Lines
augur/util_support/auspice_config.py	52.43%	68 Missing and 10 partials ⚠️
augur/export_v2.py	55.55%	5 Missing and 3 partials ⚠️
augur/util_support/warnings.py	66.66%	5 Missing and 1 partial ⚠️
augur/validate.py	45.45%	5 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1756   +/-   ##
=======================================
  Coverage   73.23%   73.24%           
=======================================
  Files          79       81    +2     
  Lines        8402     8498   +96     
  Branches     1715     1732   +17     
=======================================
+ Hits         6153     6224   +71     
- Misses       1960     1981   +21     
- Partials      289      293    +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[victorlin]

Only reviewed the first 2 commits so far, sending in comments before lab meeting

[genehack]

We could additionally validate the merged config if people want to, although the current implementation guarantees that valid configs produce a valid merged config.

I think re-validating the merged config is a good idea — even if it's the equivalent of a "this never happens" else clause in a conditional — because if somehow an invalid config was produced, I have to imagine it would be extremely confusing to debug.

[j23414]

Only commenting from a user perspective, not coding:

+1 For this feature, because I'd like to use it to simplify dengue's set of auspice configs

merging of lists is done by extending the original list, rather than simply replacing it.

Cool and on-board with this! And I imagine from a user's perspective, a usage case would be similar to

augur export --auspice-config auspice_config_base.json  add-serotype-specific-coloring-and-configs.json ...
augur export --auspice-config auspice_config_base.json  add-genotype-specific-coloring-and-configs.json ...

[jameshadfield]

Force pushed to address requested changes (see comments above) and rebased onto latest master.

I think re-validating the merged config is a good idea

Done, and also added a --output-auspice-config argument as discussed in dev chat earlier today

Read The Docs is failing as it failed to check out the branch, which is unrelated to the changes in this PR. All other CI checks pass, so it's ok to merge from that perspective.

[joverlee521]

The general merge behavior makes sense to me! My only blocking comment is to add some documentation to the help message to clarify how multiple Auspice configs are merged.

[jameshadfield]

We now have new CI failures after this force-push. I think the salient parts of the CI logs are completely unrelated to this PR, implying that it's a flake8 version bump, so we should see this appearing on other PRs imminently 🫠

tests/test_flake8.py::test_flake8 ./augur/filter/include_exclude_rules.py:973:9: F824 `nonlocal replacements` is unused: name is never assigned in scope
./build/lib/augur/filter/include_exclude_rules.py:973:9: F824 `nonlocal replacements` is unused: name is never assigned in scope
FAILED

=================================== FAILURES ===================================
_________________________________ test_flake8 __________________________________
tests/test_flake8.py:9: in test_flake8
    assert result.returncode == 0, "flake8 exited with errors"
E   AssertionError: flake8 exited with errors
E   assert 1 == 0
E    +  where 1 = CompletedProcess(args=['flake8'], returncode=1).returncode
        result     = CompletedProcess(args=['flake8'], returncode=1)

I didn't see these locally with flake8 v7.1.2, but upgrading to flake8 7.2.0 (the latest available on pip) I now see

$ python3 -m pytest --no-cov tests/test_flake8.py 
======================================================================================== test session starts =========================================================================================
platform darwin -- Python 3.11.11, pytest-8.3.5, pluggy-1.5.0 -- /Users/naboo/miniconda3/envs/augur-py11/bin/python3
cachedir: .pytest_cache
rootdir: /Users/naboo/github/nextstrain/augur
configfile: pytest.ini
plugins: cov-6.0.0, anyio-4.9.0, mock-3.14.0
collected 1 item                                                                                                                                                                                     

tests/test_flake8.py::test_flake8 ./augur/filter/include_exclude_rules.py:973:9: F824 `nonlocal replacements` is unused: name is never assigned in scope
FAILED

============================================================================================== FAILURES ==============================================================================================
____________________________________________________________________________________________ test_flake8 _____________________________________________________________________________________________
tests/test_flake8.py:9: in test_flake8
    assert result.returncode == 0, "flake8 exited with errors"
E   AssertionError: flake8 exited with errors
E   assert 1 == 0
E    +  where 1 = CompletedProcess(args=['flake8'], returncode=1).returncode
        result     = CompletedProcess(args=['flake8'], returncode=1)
========================================================================================= 1 failed in 1.04s ==========================================================================================