-
Notifications
You must be signed in to change notification settings - Fork 8
Addressed issues 192-194 #196
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple a minor things + a request to add any relevant tests for the new checks. Are all the rename-related things already split out into other issues?
Also, misc thing: it looks like git might be using attaching your computer username to the commits here, which might be a bit confusing looking at git logs. Could you please try running either: git config user.name "<Your Name>"
git config user.email "<email-addr>@<.....>.edu" in each cloned repo, or git config --global user.name "<Your Name>"
git config --global user.email "<your-email-address>@<.....>.edu" from anywhere? You can check for success using
in each repo, or
to view just the global settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once all @brookslogan comments are addressed.
We no longer import anything from vctrs, but do use it in tests; it now belongs in Suggests: rather than Imports:.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed a minor check comment. If checks pass, think this is ready to merge.
Since Issues #192, #193, and #194 are closely related, addressed all three here.
After discussion with Logan, made the following changes to
[.epi_df
code for the written issue:other_keys
or decay to tibble when selecting, renaming columns #192: Any key columns that have been selected out from anepi_df
are removed from other_keys in the metadata (refer to the ex in that issue).epi_df
totibble
, remove metadata that doesn't make sense #193: Dropped all metadata when decay to tibble.Abort
or decay to tibble ifepi_df
column duplication or renaming invalidates key #194: Abort if column duplication in anepi_df
(to be consistent with tibble behaviour).