ropensci / tidyhydat

An R package to import Water Survey of Canada hydrometric data and make it tidy
https://docs.ropensci.org/tidyhydat
Apache License 2.0
70 stars 19 forks source link

Proposed fix for issue #180, download_hydat failing to update local database. #181

Closed gdelaplante closed 1 year ago

gdelaplante commented 2 years ago

180

Primary change

hydat now gets extracted to a temp directory instead of in the directory specified by dl_hydat_here. The newly extracted database is then copied over to the dl_hydat_here location and overwrites the existing database. This circumvents the issue where file.rename() was correctly noting that the directory had two files but only one name to be assigned, and is not dependent on hydat being named in a certain manner.

Secondary changes

Rejigged the messages and the order in which messages are displayed.

In addition, the function hy_check() created at the bottom of download.R was moved up prior to the function call to hy_check(). This may not be a necessary change, but I couldn't figure out how it could work to call a function that had yet to be created!

gdelaplante commented 2 years ago

This is looking really good! Thank you for this. With respect to hy_check in an R package it doesn't usually matter where a function is defined as that all end up available once the package is loaded (either internally or as exported functions).

One thing I'd really like to do here is at least add some tests for the non-downloading behaviour. Like test for the messages. Is that something you'd be able to take on?

Again thanks so much for this. I like this logical flow much better.

I did notice that there is no test file for this function. I can pull something together for sure, however, I'm not too sure how to test all messages since some depend on there being an older version to overwrite, but should work for most. I'll also try to write a test to make sure the DB actually got updated and named properly, with no detritus in the directory. It'll take me some time to get to it though!