Skip to content

Config improvements#102

Merged
victorlin merged 6 commits intomasterfrom
victorlin/update-filter-config
Mar 6, 2026
Merged

Config improvements#102
victorlin merged 6 commits intomasterfrom
victorlin/update-filter-config

Conversation

@victorlin
Copy link
Copy Markdown
Member

@victorlin victorlin commented Sep 22, 2025

Description of proposed changes

Small adjustments to config, particularly around filtering and subsampling.

Related issue(s)

Preparing for #103

Checklist

  • Checks pass
  • Update changelog

@victorlin victorlin self-assigned this Sep 22, 2025
@victorlin victorlin mentioned this pull request Sep 22, 2025
3 tasks
Copy link
Copy Markdown
Contributor

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General config improvements make sense to me 👍

Comment thread workflow/snakemake_rules/config.smk
Comment thread workflow/snakemake_rules/core.smk
Comment thread workflow/snakemake_rules/core.smk
Comment thread Snakefile Outdated
Comment on lines +9 to +10
build_name=r"genome|G|F",
resolution=r"all-time|6y|3y",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest these should be dynamic

Suggested change
build_name=r"genome|G|F",
resolution=r"all-time|6y|3y",
build_name="|".join(config["builds_to_run"]),
resolution="|".join(config["resolutions_to_run"]),

However I then realized that these already hardcoded due to the values in the config/distance_maps.tsv. Probably good to add a note here and add to config validation in the future.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've dropped 01b4ae8 since the newly merged bc8a493 added different constraints:

rsv/Snakefile

Lines 15 to 16 in 0bafeef

build_name="|".join(config.get("builds_to_run", ["genome"])),
resolution="|".join(config.get("resolutions_to_run", ["all-time"])),

The new ones don't seem quite right, but I'll leave it as-is in this PR.

@victorlin victorlin force-pushed the victorlin/update-filter-config branch from 3340439 to 7dd63f2 Compare February 21, 2026 03:04
subrepo:
  subdir:   "shared/vendored"
  merged:   "37cf39c"
upstream:
  origin:   "https://github.com/nextstrain/shared"
  branch:   "main"
  commit:   "37cf39c"
git-subrepo:
  version:  "0.4.9"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "4f60dd7"
Useful for debugging.
augur filter has never taken a reference file as input, so my guess is
this was originally copied by mistake.
Preparing to make changes to the config that would have broken this. It
seems fine to hardcode along with the already hardcoded sample size.
This will make it easier to compare changes in the switch to augur
subsample which will define all parameters in config.
This shouldn't rely on config from another rule.
@victorlin victorlin force-pushed the victorlin/update-filter-config branch from e243d2f to 0b22185 Compare March 6, 2026 01:48
@victorlin victorlin merged commit 38f799e into master Mar 6, 2026
3 checks passed
@victorlin victorlin deleted the victorlin/update-filter-config branch March 6, 2026 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants