ropensci / skimr

A frictionless, pipeable approach to dealing with summary statistics
https://docs.ropensci.org/skimr
1.11k stars 79 forks source link

Make it flexible to combine or separate function lists for types #346

Closed elinw closed 5 years ago

elinw commented 6 years ago

As discussed in #342 ideally it should be easy for users to decide to group (or ungroup) types. The obvious examples in skimr are numeric + integer (currently separate) and factor+ Ordered factor (currently together) but there definitely will be others.

michaelquinn32 commented 6 years ago

This is pushing the limits of S3. Objects have classes, and methods are dispatched according to those classes. Inheritance is based on the order of the entries in the class attribute. https://adv-r.hadley.nz/s3.html

Still, S3 has the unusual behavior of treating two fundamentally different types (double and integer) as the same thing. It's obviously confusing and not helpful.

We have some examples already in place to derive new skimmers from old skimmers: https://github.com/ropensci/skimr/blob/3fb41691511724e1fe6f4bd841e9ed5ca82afa8e/R/get_skimmers.R#L72

Maybe a helper function would make this derivation easier.

michaelquinn32 commented 6 years ago

It's shouldn't be closed

elinw commented 6 years ago

This was a consequence of a switch that was made. But right now in v1 we have

https://github.com/ropensci/skimr/blob/master/R/functions.R#L194

https://github.com/ropensci/skimr/blob/master/R/functions.R#L228

And for ordered we just report https://github.com/ropensci/skimr/blob/master/R/functions.R#L167

Because we don't support ordered as a class skim(CO2) just treats CO2$Plant as a unordered factor.

elinw commented 5 years ago

This is fixed in v2 at this point so I'm going to close.