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

Allow feature independent stratifications in $key_join_features #203

Merged
merged 12 commits into from
Feb 26, 2025

Conversation

RasmusSkytte
Copy link
Contributor

@RasmusSkytte RasmusSkytte commented Jan 30, 2025

Intent

This PR removes a error message that blocks a specific use-case needed for diseasy.

Specifically, if you have stratifications like

rlang::quos(age_group = "0+")

then, diseasystore will currently throw an error since no features are included in the computation.

This PR removes this error

Approach

We now try to perform the grouping at the stratification level and then check if an error is produced.

Also, I had to fully deprecate non-rlang::quos() stratifications.
The documentation has specified to use rlang::quos() for a while but we still allowed it -- that is no longer the case

Known issues

N/A

Checklist

  • The PR passes all local unit tests
  • I have documented any new features introduced
  • If the PR adds a new feature, please add an entry in NEWS.md
  • A reviewer is assigned to this PR

Sorry, something went wrong.

@RasmusSkytte RasmusSkytte force-pushed the fix/non-computing-key_joins branch from 17ab2b2 to 21b59d0 Compare January 30, 2025 09:04
@RasmusSkytte RasmusSkytte self-assigned this Jan 30, 2025
@RasmusSkytte RasmusSkytte added the bug Something isn't working label Jan 30, 2025
@RasmusSkytte RasmusSkytte added this to the v0.3.1 milestone Jan 30, 2025
Copy link

codecov bot commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.50%. Comparing base (cc6c12c) to head (e7f180d).
Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #203      +/-   ##
==========================================
+ Coverage   88.44%   88.50%   +0.05%     
==========================================
  Files          18       18              
  Lines        1203     1209       +6     
==========================================
+ Hits         1064     1070       +6     
  Misses        139      139              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RasmusSkytte RasmusSkytte force-pushed the fix/non-computing-key_joins branch from 6197244 to ed294db Compare January 30, 2025 11:41
@RasmusSkytte RasmusSkytte requested review from a team, kaare-gr, SofiaOtero and LasseEngboChr and removed request for a team January 30, 2025 12:33
@RasmusSkytte RasmusSkytte force-pushed the fix/non-computing-key_joins branch 2 times, most recently from 5dd6b34 to 17b3e91 Compare February 20, 2025 07:48
@RasmusSkytte RasmusSkytte force-pushed the fix/non-computing-key_joins branch from 431ad24 to e7f180d Compare February 26, 2025 09:39
@RasmusSkytte RasmusSkytte merged commit e4b9876 into main Feb 26, 2025
28 of 29 checks passed
@RasmusSkytte RasmusSkytte deleted the fix/non-computing-key_joins branch February 26, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants