-
Notifications
You must be signed in to change notification settings - Fork 130
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
Tabyl label #575
Tabyl label #575
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #575 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 27 27
Lines 1274 1401 +127
==========================================
+ Hits 1274 1401 +127
|
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.
Outstanding! Easy to follow, thank you. I just suggested teeny comment changes.
This is great @olivroy, sorry for delay in review. When we merge this we can see if the Codecov error #576 is fixed 😁
Agree and agree... the next release of janitor should probably be 3.0.0 given that this is a breaking change and that so much time has elapsed. I do expect it could cause pain even if it's an improvement if it breaks existing code.
Yes this is irritating. #527 would at least address the tidyverse functions doing this, I think.
Could you add that workaround to |
Co-authored-by: Sam Firke <[email protected]>
Thanks so much for the review and the kind words! I am very glad my code was easy to follow, but I have to thank you and Bill for that. janitor is one of the first packages I contributed to! It is a breaking change, but only in display. I don't think many people will pipe much after. And IIRC, the original names are kept in tabyl. Let me know if you want me to add other documentation, or tests, or do more verification! |
This will work well with #564 as in my experience, names when you read from Excel in particular tend to be more human readable, the ability to keep those and then use them automatically for |
Description
Addresses #563 (comment)
SImply default to label attribute if it exists when displaying tabyl.
It enables this by default, which is a breaking change? But I wouldn't see why you would not want that if you took care of labelling your variables.
More fiddling required than expected since a lot of base R functions drop attributes
subset()
,split()
,as.character()
, etc.A solution to get the previous behavior is the following (or haven::zap_labels() or similar.
Related Issue
Partially addresses #394. Doesn't solve the value labels part, only variable label.
Example