ropensci / skimr

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

Create print.one_skim_df, document reassign_skim_attrs and modify re… #703

Open elinw opened 2 years ago

elinw commented 2 years ago

…concile_skimrs in response to changes in 4.2 and R CMD CHK issues.

702

elinw commented 2 years ago

As stated in the issue, we were getting an "object of length > 1" warning but not seeing it, now it is an error. Also pkgdown was not building (in an issue that other packages had) where there was no print function defined for an object we were printing. @michaelquinn32

We need to do a release ASAP but I'm not sure why this wasn't caught by testing on the develop branch.

elinw commented 2 years ago

So overall the initial issue was that there was no print.one_skim_df. Then that then started the issue of reconcile_skimmers not working because of the warning now being an error.
Then it could not find the reassign_skim_attrs() and I tried many things but the one that worked was removing that tag.
Don't know why, or if that means we have no examples or tests that hit the other places with that tag.

elinw commented 2 years ago

@michaelquinn32 I think I replied to at least some of your comments can you take a look?

The main issue is that we were generating a warning and now it's an error. We just didn't realize there was a warning.

michaelquinn32 commented 2 years ago

Sorry to keep pushing back, but I'm not sure this is still an issue that needs to be fixed. For example, here is the most recent r-devel check against our main branch. I don't see a warning. https://github.com/ropensci/skimr/actions/runs/3221419469/jobs/5269329413#step:6:36

I just put together a dev environment on a machine with R 4.2.1, and I didn't see any issues there either. Do you have an example where we have an issue with the current main or develop branch? If you can provide one, I can take another look.

Thanks! And I appreciate the patience with all of this.