Historic ask to update - Githubissues

bcgov / rems

An R package to access data from British Columbia's Environmental Monitoring System

Apache License 2.0

19 stars 5 forks source link

Historic ask to update #69

Closed aylapear closed 2 years ago

aylapear commented 2 years ago

update to deal with comments in shinyrems issues https://github.com/bcgov/shinyrems/issues/126 and https://github.com/bcgov/shinyrems/issues/118
add option that the function asks if you want to download the newer version similar to 2yr/4yr option
kept syntax of code similar to get_ems_data()

ateucher commented 2 years ago

Thanks! I should be able to have a look this week

aylapear commented 2 years ago

Sorry Andy got tied up in a couple things the last couple weeks but will review your comments and get back to you shortly.

The intent of the changes is to make a better user experience in shiny rems. It was expressed that the historic data is now updated daily when it used to be monthly. When a user is using the app it prompts them to download the data each day now and so an option to not re-download the new data was desired similar to how the 2yr or 4yr datasets functions. If you had just downloaded it the day before you may not want to take the time to re-download the entire historic dataset. It seemed like the current options for the historic data set didn't meet those needs exactly as the other data sets do.

aylapear commented 2 years ago

Hey Andy,

Thank you for the comments as it made me take a step back and look at a better way of structuring the code. The code is organized a bit differently now so let me know if you have any further comments or suggestions. I was trying to be quick and match more of the structure of the 2yr/4yr code but it made things more confusing and bloated then just trying to structure it for the historic setup.

This change greatly improves functionality for the shinyrems app as it allows the user to know how current their data is and whether it is worth updating it vs before you only had the option to update it if it was out of date.

> shinyrems::run_ems_app("historic")
checking for historic data...
Your version of the historic ems data is dated 2022-02-16 05:57:00 and there is a newer version available. Would you like to download it? (y/n)n
Loading required package: shiny

Listening on http://127.0.0.1:5762

ateucher commented 2 years ago

Thanks very much @aylapear for the rethinking and refactoring! It is nice and clean now - I'm just waiting for the CI to finish and then will be happy to merge!

aylapear commented 2 years ago

@ateucher I added those no cov lines which I was hoping will fix the issue with the failing action due to the code coverage

ateucher commented 2 years ago

All green! Thanks so much @aylapear for the nice work and your patience with my slow reviews 😄