Restate the baseline test cases to avoid unintended testing warnings

[Patrikios]

The test cases are built upon data frames where date is mostly of numeric mode, which shoots up (correctly, it is desired and intended in the code) the following warning message:

Warning (testneatRanges_combine.R:108:3): combine_ranges functions with date formats and startendAttr
Be aware: Column 'end' is not a date class variable, hence trying to convert into a date. Originally, the column is of mode 'numeric'. Was counting in ideal case with non-numeric mode of the column (e.g. 'character' or 'POSIXct')

As take a look at the warning:

Warning (testneatRanges_fill.R:177:3): fill_ranges functions with timestamp formats
Dimension 'timestamp' selected but format unchanged. Will try to convert to '%Y-%m-%d %H:%M:%OS' ..

can it be avoided by proper column class?

[arg0naut91]

Thanks for flagging those cases. One way would be to wrap them into suppressWarnings, like they are in the case of collapse_ranges:

df <- suppressWarnings(collapse_ranges(df_collapse, c("id", "rating"), "start_date", "end_date"))

We could also change the column classes but at least in the current cases they're there intentionally. For example, it used to be a relatively common occurrence that people were passing dates as factors to functions. This is why I put stringsAsFactors = TRUE to many test data frames - to catch any errors related to that approach already within R checks. With R 4.0.0 (and stringsAsFactors = FALSE used by default in data.frame calls) this could be a bit obsolete, however I think plenty of people are still using older versions.

For the last warning you mention, we could specify the format in the function call and the warning would disappear, regardless of the column class.

[arg0naut91]

One thing regarding suppressWarnings though - it doesn't focus on any particular warning. It'd be good in the future to try to suppress only warnings expected from our side. I'm not really aware of such functionalities at the moment though (without additional dependencies).

[Patrikios]

Yes, good point. I think as well we should just wrap, for testing purposes, suppressWarnings sensitively , when there is no test intended for those. (I can take care of than at the end of this week).

Other idea for the package quality would be to implement check (by a function/class...e.g. is_valid_range_frame) to test ultimately if correct data is being passes into the functions, which gives more control over user friendly messaging on issues (and hence boost acceptance). Then we can reuse those in all other functions.

[arg0naut91]

Yes, good point. I think as well we should just wrap, for testing purposes, suppressWarnings sensitively , when there is no test intended for those. (I can take care of than at the end of this week).

That'd be great, thanks. And I'll try to think of some scalable ways to include custom suppression.

Other idea for the package quality would be to implement check (by a function/class...e.g. is_valid_range_frame) to test ultimately if correct data is being passes into the functions, which gives more control over user friendly messaging on issues (and hence boost acceptance). Then we can reuse those in all other functions.

I agree that we'd benefit from that. I'm quite unsure about the best approach though, especially with dates/timestamps - I'm open to any suggestions here.

One thing about the following warning:

Warning (testneatRanges_combine.R:108:3): combine_ranges functions with date formats and startendAttr
Be aware: Column 'end' is not a date class variable, hence trying to convert into a date. Originally, the column is of mode 'numeric'. Was counting in ideal case with non-numeric mode of the column (e.g. 'character' or 'POSIXct')

Isn't POSIXct also of mode numeric? I think we'd need to reformulate a bit the message - or maybe just exclude the reference to POSIXct?