Add non-ATTAINS catchments to GetATTAINS, error handling updates

[kathryn-willi]

This PR modifies GetATTAINS() to include an option for "filling in" NHD catchments for WQP observations that do not have any ATTAINS assessment unit(s) associated with them. This option required pretty substantial reworking of the function and the structure of what it returns, mostly because it produces a lot of different combinations of arguments (and therefore a lot of if_else'ing was needed). This also required making a new function, fetchNHD(), which downloads the NHD catchment features. Another downstream consequence of adding this filling in option was the need to modify TADA_ViewATTAINS() to handle the new "without_ATTAINS_catchments" features.

I also made some changes to fetchATTAINS() in hopes of speeding up the API pull for grabbing that ATTAINS data. Basically, if there are very few (<100) WQP sites and the bounding box of those sites is massive (about the size of California) -- or the bounding box is a tad smaller (think size of New Hampshire), but far less observation locations (25ish), it is quicker to go site by site, as opposed to doing a single batch pull from the API.

Lastly, I added new language in our geospatial vignette (Module 2) to address the changes to the functions.
Though this does not fully close issue 441 (#441), it is getting closer. I would still like to come up with a way for pulling in the hexagonal features that are used instead of catchments for large bodies of water (think coastal AUs, the Great Lakes, etc.).

Please also stay tuned for an additional PR incorporating "WaterType" as a returned column to ATTAINS features per the new issue, "Adding assessment unit type to GetATTAINS() features" (#520).

Looking forward to your feedback!

-Katie and the ROSSyndicate team

[cristinamullin]

There is an issue with loading the arcgis package since it is not available on CRAN anymore. I think we can still use it, but it needs to go under remotes. This is causing issues with using arcgislayers as well since it is normally downloaded as a dependency of arcgis. We may need to add arcgislayers and arcpbf under "Remotes:" in the description file as well so that TADA_GetATTAINS will run without error.

https://cran.r-project.org/web/packages/arcgis/index.html

https://cran.r-project.org/web/packages/arcgislayers/index.html#:~:text=Package%20'arcgislayers'%20was%20removed%20from,from%20the%20check%20results%20archive.

https://cran.r-project.org/web/packages/arcpbf/index.html

Remotes: github::R-ArcGIS/arcgis

[cristinamullin]

@kathryn-willi I haven't been able to resolve this dependency issue with arcgislayers. It looks like we have two options: 1) we can include the code from arcgislayers directly in TADA instead of importing that package, or 2) we wait for them to get that package back on CRAN (not sure how long that might be, but it only just fell off CRAN last month).

https://stackoverflow.com/questions/59549652/how-can-a-package-on-cran-import-a-package-not-on-cran

[cristinamullin]

@kathryn-willi should this be commented out as well?

comment out TADA_ViewATTAINS lines 1485-1493 and then run devtools::document

[kathryn-willi]

oh goodness, that was definitely an accident! Will work on pushing these changes to this PR ASAP!

[kathryn-willi]

Hi @cristinamullin, the previous errors should now be sorted out. Please let me know if you encounter any more and apologies for the mistakes earlier!

[kathryn-willi]

I'm not totally sure this is needed? But did this as a precaution!

[mhweber]

These all look like great contributions! I had one question about realizations / units returned for fetchNHD - it appears now just built to return catchments but in description of function seemed that intent was to allow choice of including flowlines and waterbodies too? That would be useful I think. I believe some of what I suggested in #492 and was starting PR for could leverage adjustments made in this PR - in any event, anything I would do in a PR for that issue I would make sure to dovetail with work in this PR.

[cristinamullin]

@kathryn-willi Your updates look great - thank you for all your hard work! I created a new PR which includes all updates from this PR & in addition pulls in upstream changes from develop/handles the merge conflicts. Is there anything else you wanted to add before this is merged in?

[kathryn-willi]

@kathryn-willi Your updates look great - thank you for all your hard work! I created a new PR which includes all updates from this PR & in addition pulls in upstream changes from develop/handles the merge conflicts. Is there anything else you wanted to add before this is merged in?

Hi @cristinamullin , if @mhweber is happy with the newest commit I made (being able to pull in NHD flowlines and waterbodies as well as catchments in fetchNHD()) then this should be it for now! I plan to submit a separate PR with some updates that address #520 in the next week or so - should be a tighter, more succinct PR. :)

[mhweber]

Hi @cristinamullin , if @mhweber is happy with the newest commit I made (being able to pull in NHD flowlines and waterbodies as well as catchments in fetchNHD()) then this should be it for now! I plan to submit a separate PR with some updates that address #520 in the next week or so - should be a tighter, more succinct PR. :)

@kathryn-willi Yes, looks great!

[cristinamullin]

created a new PR that include these updates & resolves merge conflicts: #527