Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filter: Expand subsampling docs #1425

Merged
merged 4 commits into from
Mar 5, 2024
Merged

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Feb 23, 2024

(preview)

Description of proposed changes

Expand subsampling docs with a guide on how to implement multi-pass subsampling¹.

¹ internal subsampling doc definition of "multi-pass subsampling": Subsampling done as multiple calls to a subsampling tool, where intermediate subsamples are created and joined together to create a final/combined sample. It is used to work around limitations on what can be done in a single pass.

Related issue(s)

Checklist

@victorlin victorlin self-assigned this Feb 23, 2024
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
@victorlin victorlin marked this pull request as ready for review February 24, 2024 00:54
@victorlin victorlin requested a review from a team February 24, 2024 00:54
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@huddlej huddlej left a comment

Choose a reason for hiding this comment

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

These docs have been needed for so long, so thank you for making them a reality, @victorlin. With the exception of some minor typos that @jameshadfield pointed out, you could easily merge this now and it would be a big help to users. I made a few comments below that mostly attempt to clarify the content for new users.

docs/usage/cli/filter.rst Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
@huddlej
Copy link
Contributor

huddlej commented Feb 26, 2024

As a thought experiment, it would be interesting to see how we'd implement these same augur filter subsampling examples with the proposed augur subsample command and config file. I guess it would look a bit like the subsampling block of the build config YAML?

docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
@victorlin
Copy link
Member Author

I've resolved all conversations above. Will merge on any approval.

Copy link
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.

Thank you for the clear and thoughtful docs! It will be nice to be able to point users to these guides 🙏

I found one more typo and left some other non-blocking comments, but I think good to merge whenever you think it's ready.

docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
docs/usage/cli/filter.rst Outdated Show resolved Hide resolved
- Use section headings.
- Remove references to zika-tutorial. It's largely independent of that
  and prone to getting out of sync.
- Use --output-sequences and --output-metadata.
- Fix indentation.
@victorlin victorlin merged commit bd6be41 into master Mar 5, 2024
20 checks passed
@victorlin victorlin deleted the victorlin/subsampling-docs branch March 5, 2024 01:35
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.

4 participants