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.

Sign up for GitHub

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

Jump to bottom

Create a rename() function that is more specialized for an epi_df #195

Open
rachlobay opened this issue Aug 8, 2022 · 1 comment
Open

Create a rename() function that is more specialized for an epi_df #195

rachlobay opened this issue Aug 8, 2022 · 1 comment
Assignees
@brookslogan
Labels
P2 low priority

Comments

@rachlobay
Copy link
Collaborator

rachlobay commented Aug 8, 2022 •
edited
Loading

Want to create our own rename() function such that if we rename a column corresponding to other_keys, we update the metadata accordingly (to have the updated name). For example, if we consider ex3 from here, if the user performs renamed_ex3 = ex3 %>% dplyr::rename(Pol = pol), then the metadata for renamed_ex3 should be updated to have Pol under $other_keys instead of pol. Also, if the user tries to change the name of either geo_value or time_value, should probably throw an error as those two are important to leave named as they are. Mentioned in Issues #192 and #194.
@rachlobay rachlobay added the P2 low priority label Aug 8, 2022
@rachlobay rachlobay changed the title Create our own implementation for rename() Create a rename() function that is more specialized for an epi_df Aug 8, 2022
@rachlobay rachlobay mentioned this issue Aug 11, 2022
Merged
@brookslogan
Copy link
Contributor

Related: #223. ?dplyr_extending might provide a way to limit the number of S3 implementations we have to provide.

@brookslogan brookslogan self-assigned this Nov 1, 2022
brookslogan pushed a commit that referenced this issue Nov 1, 2022
@lcbrooks
Address failing test, use dplyr_extending, don't ungroup in slides
cfebe5f 
* Fix failing grouped `epix_slide` test, where `group_modify` dropped the
  `epi_df` class when binding results together, by implementing
  `dplyr_reconstruct.epi_df`.
* Implement the rest of `?dplyr_extending` and remove some now-unnecessary S3
  methods for dplyr verbs, addressing #195, #223, and changing `epi[x]_slide` to
  leave grouping intact.
* Update tests for grouped slides to reflect new behavior.
* Update NEWS.md.
brookslogan pushed a commit that referenced this issue Nov 1, 2022
@lcbrooks
Half-fix failing test, use dplyr_extending, don't ungroup in slides
4ca96b0 
* Half-fix failing grouped `epix_slide` test, where `group_modify` dropped the
  `epi_df` class when binding results together, by implementing
  `dplyr_reconstruct.epi_df`.  Somehow a session documenting and testing
  immediately before this commit and then after it will have the tests pass, but
  the tests won't pass off of this branch directly.  The class vector involved
  in the failure does appear to change.
* Explicate `epix_slide` `count` derivation.
* Implement the rest of `?dplyr_extending` and remove some now-unnecessary S3
  methods for dplyr verbs, addressing #195, #223, and changing `epi[x]_slide` to
  leave grouping intact.
* Update tests for grouped slides to reflect new behavior.
* Update NEWS.md.
brookslogan pushed a commit that referenced this issue Nov 2, 2022
@lcbrooks
Use dplyr_extending, don't ungroup in epix_slide, back to `group_…
11f65e7 
…modify`

* Implement `?dplyr_extending` and remove some now-unnecessary S3 methods for
  dplyr verbs, addressing #195, #223, and failing `epix_slide` test.
* Don't ungroup `epix_slide` result.  Update corresponding test.
* Update NEWS.md.
* Explicate `epix_slide` `count` derivation in comments and variable names.
* Fix some desynced duplicated code in `epix_slide` and use `group_modify` again
  instead of `summarize` in order to keep slide computation input available as
  an `epi_df`.
brookslogan pushed a commit that referenced this issue Nov 2, 2022
@lcbrooks
Use dplyr_extending, don't ungroup in epix_slide, back to `group_…
0ffcff5 
…modify`

* Implement `?dplyr_extending` and remove some now-unnecessary S3 methods for
  dplyr verbs, addressing #195, #223, and failing `epix_slide` test.
* Don't ungroup `epix_slide` result.  Update corresponding test.
* Update NEWS.md.
* Explicate `epix_slide` `count` derivation in comments and variable names.
* Fix some desynced duplicated code in `epix_slide` and use `group_modify` again
  instead of `summarize` in order to keep slide computation input available as
  an `epi_df`.
brookslogan pushed a commit that referenced this issue Nov 3, 2022
@lcbrooks
Use dplyr_extending, don't ungroup in epix_slide, back to `group_…
10d5f55 
…modify`

* Implement `?dplyr_extending` and remove some now-unnecessary S3 methods for
  dplyr verbs, addressing #195, #223, and failing `epix_slide` test.
* Don't ungroup `epix_slide` result.  Update corresponding test.
* Update NEWS.md.
* Explicate `epix_slide` `count` derivation in comments and variable names.
* Fix some desynced duplicated code in `epix_slide` and use `group_modify` again
  instead of `summarize` in order to keep slide computation input available as
  an `epi_df`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brookslogan brookslogan

Labels
P2 low priority
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@brookslogan @rachlobay