Skip to content

phylo: Standard metadata id field#108

Merged
joverlee521 merged 2 commits into
mainfrom
standard-metadata-id
Jul 9, 2026
Merged

phylo: Standard metadata id field#108
joverlee521 merged 2 commits into
mainfrom
standard-metadata-id

Conversation

@joverlee521

Copy link
Copy Markdown
Contributor

Description of proposed changes

Update to our new standard of defining inputs with id_field and expecting all downstream Augur commands to use id from nextstrain/zika#110.

Related issue(s)

Part of nextstrain/public#44

Checklist

  • Checks pass

joverlee521 and others added 2 commits July 8, 2026 14:14
Update to our new standard of defining inputs with `id_field` and
expecting all downstream Augur commands to use `id` from 
<nextstrain/zika#110>

Part of <nextstrain/public#44>

Co-authored-by: Victor Lin <vlin@fredhutch.org>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thoughts on moving this code to a Snakemake module for reusability?

module merge_inputs:
    snakefile: "../../shared/vendored/snakemake/merge_inputs.smk"
    config: config

use rule merge_metadata from merge_inputs with:
    output:
        metadata = "results/metadata.tsv"

use rule merge_sequences from merge_inputs with:
    output:
        sequences = "results/sequences.fasta"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's been a while since I've looked at modules. I think this could work now that we only need the merge rule without caring about the number of inputs...

My initial question is how to handle the _gather_inputs function since that may differ per pathogen (e.g. avian-flu handles segment sequences). Reading through the module docs again, I'm remembering that it's not possible to override a function within a module. I think we would need to pass in the input_sources variable via the config to the Snakemake module.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Even if this gets weird for some repos like avian-flu, those repos can continue to keep their own version of merge_inputs.smk.

But the current code should work for most repos, and I wonder if the idea can be extended to other rules as well...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Even if this gets weird for some repos like avian-flu, those repos can continue to keep their own version of merge_inputs.smk.

Ah, that's true! It's definitely an opt-in

But the current code should work for most repos, and I wonder if the idea can be extended to other rules as well...

I think it'd be nice to do for completely standardized steps like the initial merge of inputs. Other rules might be more up for debate (based on older discussions of modules)...I imagine pivoting to config based augur commands could also bring us to modules since the augur rules would essentially be identical across pathogens.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah. It can be considered separately from this PR though!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@joverlee521 joverlee521 merged commit 0affc92 into main Jul 9, 2026
1 check passed
@joverlee521 joverlee521 deleted the standard-metadata-id branch July 9, 2026 18:08
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