-
Notifications
You must be signed in to change notification settings - Fork 8
Lcb/slide unnest dedupe cols #509
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
Merged
brookslogan
merged 11 commits into
lcb/slide-improvements-2024-06
from
lcb/slide-unnest-dedupe-cols
Sep 4, 2024
Merged
Lcb/slide unnest dedupe cols #509
brookslogan
merged 11 commits into
lcb/slide-improvements-2024-06
from
lcb/slide-unnest-dedupe-cols
Sep 4, 2024
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
74241a8
to
ab0c50a
Compare
86f8342
to
d759628
Compare
…ames In `epix_slide()`: - warn-deprecate `.ref_time_values =` in favor of `.versions =` - allow tidyeval or formula comps to use `.ref_time_value` or `.version` to access the ref_time_value/version (currently, these two things are always the same) - output a `version` column, not a `time_value` column - rename `epix_slide_ref_time_values_default` -> `epix_slide_versions_default` - some other cleanup from a rebase combining with dot-prefixing and other slide changes
- Default is now to not mark any versions as clobberable; simply remove discussion of old default as it was to explain a surprise/annoyance in normal use. - Favor using `$versions_end` to get the latest version; while in examples it's probably similar, in general, it's more "correct" and should be faster.
since this seems like more appropriate and consistent naming for the main use case of extracting an `epi_df` snapshot.
Collapse with empty string in order to not have extra whitespace if used with `cat` rather than `cli_*`.
Forbidding `new_col_name` being among the labeling columns addresses some dedupe cases where deduping would always lead to failure except for completely-redundant computations (that only output computation labels rather than and actual computation). - This might not be complete in a edge case where `"slide_value"` is a grouping variable. (E.g., from using a slide to assign a categorical trend, then doing a grouped slide based on the trend.) This is definitely only part of the dedupe handling. Unpacked-column outputs need to actually be de-duped. Also, fix incorrect documentation for time_value filter for .all_versions = TRUE while rebasing on other slide updates.
ce5b91f
to
9127952
Compare
Since we're passing along ... from outer fns to our inner helper fns taking ..., the internal fns should also dot-prefix if outer should.
2eabcc0
to
1181b97
Compare
[Apparently I merged this already somehow? And we thought this was a draft but somehow wasn't? Which explains why I didn't see it and needed to open #521 and thought I had somehow pushed some stuff directly to the slide improvements mega-PR... Removing redundant checklist here.] |
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
Please:
PR).
brookslogan, nmdefries.
DESCRIPTION
. Always incrementthe patch version number (the third number), unless you are making a
release PR from dev to main, in which case increment the minor version
number (the second number).
(backwards-incompatible changes to the documented interface) are noted.
Collect the changes under the next release number (e.g. if you are on
1.7.2, then write your changes under the 1.8 heading).
process.
Change explanations for reviewer
Magic GitHub syntax to mark associated Issue(s) as resolved when this is merged into the default branch