search

prioritizr / wdpar

Interface to the World Database on Protected Areas
https://prioritizr.github.io/wdpar
GNU General Public License v3.0
39 stars 5 forks source link

JOSS Review warning about out of date local data #59

Closed DrMattG closed 2 years ago

DrMattG commented 2 years ago

Warning message: In wdpa_fetch("Norway", wait = TRUE, download_dir = rappdirs::user_data_dir("wdpar")) : local data is out of date: jul 2022 (https://github.com/prioritizr/wdpar/blob/40e8e79501b7caf16f684ec0f386a6299b241d07/R/wdpa_fetch.R#L183) What does it mean if the local data is out of date? Do I need to worry about this warning? It would be good to have some comment on this in the documentation (or somewhere). https://github.com/openjournals/joss-reviews/issues/4594

jeffreyhanson commented 2 years ago

The package will, by default, load the latest version of the database that is available on your computer (e.g., because you previously downloaded the data). This warning message is telling you that this version is now out of date, and a newer version is available online. I guess you would need to worry about this warning if you are running analyses that require the latest version, but generally this isn't the case. Sure, I can add some info to the docs on this message.

jeffreyhanson commented 2 years ago

Sorry for the slow progress on this @DrMattG. Just to keep you in the loop, I'm planning on creating a PR to incorperate this feedback - once I've merged the PR to address the issues raised by @Jo-Schie. Since I'm really bad at dealing with merge conflicts, I'd rather not create multiple branches at the same time and then have to juggle conflicts when merging them. Did you have any other issues/concerns (apart from #58 and #59) as part of your review? Or will I have addressed all your major concerns once these are addressed? Thanks for your patience!

DrMattG commented 2 years ago

Hi @jeffreyhanson - That's fine - I think I am happy with everything else - I understand the merge conflict issue!

jeffreyhanson commented 2 years ago

Ok awesome - thank you!

jeffreyhanson commented 2 years ago

@DrMattG, I've just created a PR to address this issue. When you get a chance, could you please take a look? I've put the changes specifically related to this issue in this commit: https://github.com/prioritizr/wdpar/pull/64/commits/c83514c2fe4ef1bcc325549270f52975c07f4500.

jeffreyhanson commented 2 years ago

Woops, forgot tag this issue in the commit when merging this PR, so I'll close this manually.