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

Grouping, Printing and Formatting #122

Closed
wants to merge 73 commits into from
Closed

Grouping, Printing and Formatting #122

wants to merge 73 commits into from

Conversation

elinw
Copy link
Collaborator

@elinw elinw commented Jun 28, 2017

This greatly simplifies printing and address some of the issues around formatting/significant digits.
The main change is to add a "formatted_value" column so that now levels is only used for levels, and formatted values for printing (and other purposes) are in the new column. This includes both the spark graphs and the dates. If no specific formatted value is declared in the statistic the stored value is converted to a character and used. This means that the display does not add extra 0s. However calculated values at this point still are displaying as is. However those issues will now be easier to address in a flexible way.
A user could define their own formatting using colformat, lucid or their own approach.
I think we should still come up with a default solution for mean and standard deviation but that's a lot simpler than having to do everything.

Some of the tests are failing (adding the extra column required a lot of changes).
screen shot 2017-06-27 at 10 00 24 pm
screen shot 2017-06-27 at 9 58 55 pm
screen shot 2017-06-27 at 9 58 03 pm

GShotwell and others added 30 commits May 29, 2017 14:21
Print method does not work properly
Print method does not work properly
Printed out value of grouping variable


Included grouping variable value
This is to match the output of dplyr::summarize().
import from magrittr not dplyr
Fixed some items that Travis was reporting.
Also covr has a known issue of misreporting coverage of lists of functions, so excluding those (which are all tested) from the report.
@codecov-io
Copy link

codecov-io commented Jun 29, 2017

Codecov Report

Merging #122 into master will increase coverage by 7.97%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #122      +/-   ##
========================================
+ Coverage   92.02%   100%   +7.97%     
========================================
  Files           5      6       +1     
  Lines         163    200      +37     
========================================
+ Hits          150    200      +50     
+ Misses         13      0      -13
Impacted Files Coverage Δ
R/functions.R 100% <ø> (+8.69%) ⬆️
R/skim_v.R 100% <100%> (ø) ⬆️
R/print_handlers.R 100% <100%> (ø)
R/skim_print.R 100% <100%> (+17.46%) ⬆️
R/stats.R 100% <100%> (ø) ⬆️
R/skim.R 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eea37e2...55d3198. Read the comment docs.

@elinw elinw changed the title Formatting concept for discussion Grouping, Printing and Formatting Jul 16, 2017
@elinw
Copy link
Collaborator Author

elinw commented Sep 1, 2017

This is all now in the develop branch.

@elinw elinw closed this Sep 1, 2017
@elinw elinw deleted the formattingconcept branch October 13, 2017 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants