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

Refactor subsampling into its own function #746

Merged
merged 1 commit into from
Jul 16, 2021
Merged

Conversation

huddlej
Copy link
Contributor

@huddlej huddlej commented Jul 10, 2021

Description of proposed changes

Adds a new function subsample with the logic originally defined in run. This new function allows us to reorganize the run function more substantially and confidently split the subsampling logic into more manageable components.

Related issue(s)

Builds on #743 and #745.
Related to #699.

Testing

No additional testing besides CI tests.

@codecov
Copy link

codecov bot commented Jul 10, 2021

Codecov Report

Merging #746 (1983837) into master (2482992) will decrease coverage by 0.01%.
The diff coverage is 1.26%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #746      +/-   ##
==========================================
- Coverage   31.54%   31.52%   -0.02%     
==========================================
  Files          41       41              
  Lines        5751     5754       +3     
  Branches     1389     1389              
==========================================
  Hits         1814     1814              
- Misses       3860     3865       +5     
+ Partials       77       75       -2     
Impacted Files Coverage Δ
augur/filter.py 52.08% <1.26%> (-0.37%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2482992...1983837. Read the comment docs.

@huddlej huddlej marked this pull request as ready for review July 15, 2021 18:57
Copy link
Member

@tsibley tsibley left a comment

Choose a reason for hiding this comment

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

@huddlej and I talked about this offline. Summary is 👍, except this branch looks like it needs to be rebased onto the latest #743 and rebased version of #745.

Adds a new function `subsample` with the logic originally defined in
`run`. This new function allows us to reorganize the `run` function more
substantially and confidently split the subsampling logic into more
manageable components.
@huddlej huddlej force-pushed the refactor-subsampling branch from 86ef7a1 to 1983837 Compare July 16, 2021 02:23
@huddlej huddlej merged commit 653079e into master Jul 16, 2021
@huddlej huddlej deleted the refactor-subsampling branch July 16, 2021 04:09
@huddlej huddlej added this to the Feature release 13.0.0 milestone Jul 16, 2021
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