search

epiforecasts / NCoVUtils

Utility functions for the 2019-NCoV outbreak
https://epiforecasts.io/NCoVUtils/
Other
27 stars 13 forks source link

remove tibble dependency #54

Closed hamishgibbs closed 4 years ago

hamishgibbs commented 4 years ago

Removed tibble dependency by changing tibble::tibble to dplyr::tibble and tibble::as_tibble to dplyr::as_tibble. Also rewrote tibble::enframe in get_japan_regional_cases to rely only on dplyr.

seabbs commented 4 years ago

I think this doesn't do what is expected as dplyr depends on tibble for this functionality? Happy to be wrong

hamishgibbs commented 4 years ago

You are right, dplyr depends on tibble for tibble::tibble and tibble::as_tibble. Is it better to rely on dplyr to have tibble as a dependency?

So that we do not import tibble in this package when it is already a dependency of dplyr?

Or does that not make too much difference?

seabbs commented 4 years ago

It doesn't really make a difference our deps just go dplyr -> tibble rather than straight to tibble. In general I like to err on the side of clarity so would rather stick with dep on tibble. Sorry!

seabbs commented 4 years ago

(suggest closing 🔓 )

hamishgibbs commented 4 years ago

That sounds good, no need to use tibble through dplyr!