refine: Implement --config by victorlin · Pull Request #2040 · nextstrain/augur

victorlin · 2026-08-15T01:27:41Z

Description of proposed changes

This PR implements augur refine --config, an option similar to augur subsample --config but for augur refine's own CLI options. See commits for details and #1987 for the motivation.

The implementation was written in a way that should make it easy to add --config to other commands in the future.

Docs preview: https://nextstrain--2040.org.readthedocs.build/projects/augur/en/2040/usage/cli/refine.html#configuration

Notable review threads

Checklist

Automated checks pass
Check if you need to add a changelog message
Check if you need to add tests
Check if you need to update docs
PR nextstrain.org for schema route

codecov · 2026-08-15T01:39:49Z

Codecov Report

❌ Patch coverage is 89.65517% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.40%. Comparing base (0aed098) to head (11a6933).

Files with missing lines	Patch %	Lines
augur/argparse_.py	87.87%	6 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2040      +/-   ##
==========================================
+ Coverage   73.26%   73.40%   +0.14%     
==========================================
  Files          86       86              
  Lines       10765    10868     +103     
  Branches     2099     2123      +24     
==========================================
+ Hits         7887     7978      +91     
- Misses       2503     2509       +6     
- Partials      375      381       +6

☔ View full report in Codecov by Harness.
📢 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.

joverlee521

Kudos for designing a way to create the config schema from the parser! I left some open questions for discussion, but I do really like this direction.

joverlee521 · 2026-08-17T23:43:35Z

+    # NOTE: Nothing is required because everything can also be set in the CLI.
+    # Though, the CLI help text still says required for options that can now be
+    # set in --config.


Hmm, this will make schema validation less useful in the context of workflows...Not as much of an issue for refine since only the tree input is marked as required, but could be a problem for other commands.

I've added required in 4d91566, but might revert it per #2040 (comment).

joverlee521 · 2026-08-18T00:06:51Z

+        # Default allows command line arguments to override config file values,
+        # but we want them to be mutually exclusive.


If we want config and CLI to be mutually exclusive, should the --config option just be mutually exclusive with all other CLI options? Ah, but we would want to define the inputs/outputs via CLI in the Snakemake workflows...I see why you did it this way.

It would have been nice for the schema validation to guard against errors that we currently can only catch at runtime

augur/augur/refine.py

Lines 243 to 250 in 0aed098

if not args.alignment:

if args.timetree:

print("ERROR: alignment is required for ancestral reconstruction or timetree inference", file=sys.stderr)

return 1

if args.divergence_units=='mutations':

print("ERROR: alignment is required for divergence in units of mutations", file=sys.stderr)

return 1

I've pushed 4d91566 which makes them mutually exclusive, but might revert it per #2040 (comment).

Note that even with that commit, --alignment is marked as CLI-only so we still need the runtime checks.

jameshadfield

Really cool to see this Victor. I tried to dive as deep as I could, although argparse constantly confuses me! It was nice to see how simple this work made it to add this capability to further commands in #2041

jameshadfield · 2026-08-18T02:18:06Z

+
+        return action
+
+    def get_possible_config_keys(self, action):


Can you add a docstring? Following the upstream something like

This method decides which actions can be set in a config file and what their keys will be. It returns a list of 0 or more keys which can be used in a config YAML file.

Added:

augur/augur/argparse_.py

Lines 62 to 66 in 7838bb3

"""

This method decides which actions can be set in a config file and what

their keys will be. It returns a list of 0 or more keys which can be

used in a config YAML file.

"""

Did you want to see docstrings on other overridden methods too? I had hoped the upstream methods were good enough context for the purpose of each, and instead focused on comments explaining our custom behavior.

Thanks -- yeah, I think so on the others, but I'll leave up to you. The upstream methods are good enough once you track them down, but ideally you could read our file on it's own and have a general understanding of what's going on

jameshadfield · 2026-08-18T02:49:56Z

+
+        return action
+
+    def get_possible_config_keys(self, action):


This will expose ~all options to the config YAML. I remember some discussion about not allowing filepath inputs in the config.yaml, but I can't find nor remember the outcome of that discussion. I don't mind having filepath args in the config YAML, just wanted to note that we discussed it at somepoint.

In that discussion, I decided it'd be easier to allow everything for simplicity, even though in practice we would keep filepath inputs in CLI for Snakemake. However, the simplicity comes with drawbacks as noted by the other review comments.

I have a local draft that marks the filepath inputs (and --no-covariance) as CLI-only. Planning to push it up soon.

Keeping all available in YAML will provide benefits too tho. For instance, augur ancestral's --root-sequence is optional. If a user's config override YAML can simply add root_sequence to the corresponding block it makes it trivial to use this functionality; without it we'd have to add in custom snakemake & yaml logic.

I've pushed the changes as 4d91566 but might revert it. While it solves many of the other review comments, it forces us to make a hard decision in #2041 for whether root_sequence should be in CLI or YAML. WA DOH uses YAML, but there could be a valid use case for having it in CLI instead...

Yeah, I see the appeal of using root_sequence for optional inclusion.

I think one issue regardless of which way we go is that it's not obvious in the pathogen workflow config which params are config vs CLI. I wonder if we revert 4d91566 to make the Augur config more flexible, but then in each pathogen workflow schema, we would reference a subset of params?

I've moved parts of that commit into 564e2b5 for #2040 (comment) and will likely revert the remainder in 3182ccb.

Will look into referencing a subset of the schema in nextstrain/measles#146.

... it's not obvious in the pathogen workflow config which params are config vs CLI ... in each pathogen workflow schema, we would reference a subset of params?

Thinking through the use case where a workflow has metadata defined as an input (i.e. referencing the output of another rule) and we want to avoid a config YAML defining metadata: something_else.tsv. Currently it'll be a runtime error which I think is enough for typical workflows. You could imagine the workflow schema specifying a keys-not-allowed-in-yaml: ['metadata'] which the GUI could parse and therefore prevent the user re-defining "metadata". I'm a little wary of how much complexity doing more than this would add, but I don't have a feeling for what schema subsets will look like in practice.

Ok, I've dropped 4d91566. b0bb94e is a milder version, but I still decided not to add it.

Encoding keys-not-allowed-in-yaml: ['metadata'] using JSON schema alone is tricky, similar to something we tried in #2037. We might have to generate the schema with a script to properly subset the schema per workflow.

jameshadfield · 2026-08-18T03:12:45Z

+        "metadata": {
+            "description": "sequence metadata",
+            "type": "string"
+        },


Aside: I imagine we'll use this schema for a general get_referenced_files function. That function (I think!) wants to see filepaths declared in the schema (if prop_schema.get("format") == "filepath") which are present in the subsampling schema:

augur/augur/data/schema-subsample-config.json

Lines 60 to 65 in 0aed098

"include": {

"oneOf": [

{

"type": "string",

"format": "filepath"

},

should we be adding them here?

Good point. There's nothing in the current argparse usage that marks arguments as filepaths, so I've added a custom InputFile type for the schema generating script:

augur/devel/regenerate-config-schemas

Lines 116 to 118 in 7838bb3

elif action.type is InputFile:

prop["type"] = "string"

prop["format"] = "filepath"

One related point -- some arguments are files or non-files. At least, I think we have some of them! We can leave their handling until we encounter them tho.

Enables configuration via a YAML file in addition to CLI arguments. All options available in the CLI are also available in the YAML file, but an option cannot be set in both simultaneously. Updated help text to reference both CLI and YAML syntax, now that both are valid options. Implemented using a new dependency, ConfigArgParse. It handles the YAML configuration within argparse so that the rest of the code can continue referencing the values from argparse. Lots of behavioral customization was added in CustomArgumentParser. Note that it doesn't make sense to use ConfigArgParse with augur subsample because the YAML config used in that command has a nested structure, opposed to the flat structure implicitly supported by the CLI. This commit just adds the functionality and a test. More to come in the following commits.

This will be useful as a basis for documentation, and validation by pathogen workflows. Done with a new script that inspects the argparse parser object so that all the logic and help text can stay in register_parser(). Added a new type=InputFile for argparse so that the schema can mark certain properties as filepaths.

The schema by itself isn't very useful for users. A table describing the available options is much more decipherable. Luckily we already do something for augur subsample's config docs. I've generalized an existing helper function to work with any flat JSON schema.

`covariance: False` accomplishes the same thing in a more intuitive syntax for the config file. Done by marking the option as CLI-only using a custom param in argparse's add_argument().

This was likely always the intention, but they were allowed together without the mutually exclusive group. The injection of cli_only in CustomArgumentParser.add_argument() is no longer sufficient as previously noted. Instead of monkeypatching the underlying add_argument() call, I chose to apply the same treatment to individual custom classes – more code but less hacky.

Improve formatting of help message

afc2378

victorlin self-assigned this Aug 15, 2026

victorlin mentioned this pull request Aug 15, 2026

Config based Augur commands #1987

Open

7 tasks

victorlin force-pushed the victorlin/refine-config branch 2 times, most recently from 0d1ed80 to 8d72633 Compare August 17, 2026 17:52

joverlee521 reviewed Aug 18, 2026

View reviewed changes

victorlin mentioned this pull request Aug 18, 2026

Use augur refine --config nextstrain/measles#146

Draft

2 tasks

victorlin force-pushed the victorlin/refine-config branch from 8d72633 to c0e909f Compare August 18, 2026 00:24

jameshadfield self-requested a review August 18, 2026 00:46

jameshadfield reviewed Aug 18, 2026

View reviewed changes

victorlin force-pushed the victorlin/refine-config branch 3 times, most recently from 3182ccb to 3bdd7f3 Compare August 19, 2026 21:06

victorlin added 6 commits August 19, 2026 17:29

Add docs for --config

875c074

The schema by itself isn't very useful for users. A table describing the available options is much more decipherable. Luckily we already do something for augur subsample's config docs. I've generalized an existing helper function to work with any flat JSON schema.

Update changelog

031088b

Remove no_covariance option from config file

8ba183a

`covariance: False` accomplishes the same thing in a more intuitive syntax for the config file. Done by marking the option as CLI-only using a custom param in argparse's add_argument().

victorlin force-pushed the victorlin/refine-config branch from 3bdd7f3 to 11a6933 Compare August 20, 2026 00:36

		# Default allows command line arguments to override config file values,
		# but we want them to be mutually exclusive.

	if not args.alignment:
	if args.timetree:
	print("ERROR: alignment is required for ancestral reconstruction or timetree inference", file=sys.stderr)
	return 1

	if args.divergence_units=='mutations':
	print("ERROR: alignment is required for divergence in units of mutations", file=sys.stderr)
	return 1

	"""
	This method decides which actions can be set in a config file and what
	their keys will be. It returns a list of 0 or more keys which can be
	used in a config YAML file.
	"""

	"include": {
	"oneOf": [
	{
	"type": "string",
	"format": "filepath"
	},

	elif action.type is InputFile:
	prop["type"] = "string"
	prop["format"] = "filepath"

Uh oh!

Conversation

victorlin commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of proposed changes

Notable review threads

Checklist

Uh oh!

codecov Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joverlee521 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jameshadfield left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

victorlin commented Aug 15, 2026 •

edited

Loading

codecov Bot commented Aug 15, 2026 •

edited

Loading