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

Add filtering joins, with documentation and tests. #120

Merged
merged 3 commits into from
May 23, 2021

Conversation

bschneidr
Copy link
Contributor

This would partially resolve #65, adding two types of joins (filtering rather than mutating) which won't accidentally mess up the survey design.

I've tested the use for in-memory datasets, but I'd like to add a couple tests for databases once I can fully wrap my head around how to get those tests running.

@codecov-commenter
Copy link

codecov-commenter commented May 12, 2021

Codecov Report

Merging #120 (7f2130c) into main (7c2c77c) will increase coverage by 0.34%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #120      +/-   ##
==========================================
+ Coverage   78.65%   78.99%   +0.34%     
==========================================
  Files          20       21       +1     
  Lines         890      914      +24     
==========================================
+ Hits          700      722      +22     
- Misses        190      192       +2     
Impacted Files Coverage Δ
R/join.R 91.66% <91.66%> (ø)

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 7c2c77c...7f2130c. Read the comment docs.

R/join.R Outdated
na_matches = na_matches,
...)

x <- mutate(x, `___retained` = `___row_number` %in% filtered_vars[['___row_number']])
Copy link
Owner

@gergness gergness May 12, 2021

Choose a reason for hiding this comment

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

Can you skip the ___retained variable and just filter on the expression? (If not, I thnk you need to de-select the ___row_number variable, don't you?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's a good idea. I didn't de-select ___row_number because it was actually removed internally in the call to filter(). But just to be careful, in f240676 added a conditional select to remove that column if it still exists after using filter().

…so conditionally remove the `___row_number` variable if it's not already removed internally by `filter()`.
@gergness gergness merged commit 60cff29 into gergness:main May 23, 2021
@gergness
Copy link
Owner

thanks!

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.

Implement slice(), *_join(), and other dplyr methods for tbl_svy.
3 participants