Open mhweber opened 4 months ago
@mhweber - we had a chance to discuss this at our last team meeting and would welcome a PR from you on this as we don't currently have a way to support user-supplied spatial features in existing TADA mapping functions.
@hillarymarler - great! I'll try to put in a PR for this soon
@kathryn-willi FYI Marc is also working on some updates to TADA_ViewATTAINS()
@mhweber Both functionalities, allowing toggling of layers and incorporation of user-supplied spatial features, are very helpful and welcomed additions! Users have specifically asked for this - for example so they could add their up-to-date assessment units and then match those with WQP sites (instead of the ones in ATTAINS that may be are outdated). Visualizing other layers may be helpful for making decision to include/exclude a site in analyses, or may assist with assessment unit development/segmentation, or understanding/sourcing wq issues.
The ROSSyndicate lab (@kathryn-willi, @mbrousil and others) at Colorado State has been working on updates to the geospatial functions as well. See their most recent PR here: https://github.com/USEPA/EPATADA/pull/521. If you have time, I just added you as a reviewer because this overlaps with the work you’re doing. They modified both TADA_GetATTAINS and TADA_ViewATTAINS. I hope it is not too much effort to merge your updates with theirs. Depending on timing, it may may sense to merge both of you PRs before merging all updates into EPATADA develop.
TADA_GetATTAINS currently links catchment-based ATTAINS assessment unit data (EPA snapshot of NHDPlus HR catchments associated with entity submitted assessment unit features - points, lines, and polygons) to Water Quality Portal observations. The new functionality ROSSyndicate added should allow users to essentially visualize/create new assessment units that are the NHD catchments for WQP observations that do not have any ATTAINS assessment units associated with them yet. Or it would allow users to investigate how much land/what areas are “not assessed” in ATTAINS (under CWA) but do have WQP data available for assessments.
Is your feature request related to a problem? Please describe: It seems it would be useful to allow toggling layers in leaflet map on and off in TADA_ViewATTAINS() and potentially TADA_OverviewMap(). This would be moderately useful for existing mapping function(s), and could incorporate switching base maps, but would be critical if
TADA_ViewATTAINS()
or other mapping functions inEPATADA
allowed incorporation of user-supplied spatial features (which would be a great addition).Describe the solution you'd like: If it is not already in process (I didn't see) and desirable, it would be easy to show / hide layers using addLayersControl. For instance in GeospatialFunctions.R in
TADA_ViewATTAINS()
it would be easy to modify the map object with something like:I would be happy to put in a PR for this if the EPATADA team thought this would provide useful functionality. I would also be interested in contributing to efforts to incorporate user-supplied spatial features but this may already be in the works.
Reminders for TADA contributors addressing this issue:
New features and/or edits should include all the following work:
[ ] Create or edit the function/code.
[ ] Document all code using line/inline and/or multi-line/block comments to describe what is does.
[ ] Create or edit tests in tests/testthat folder to help prevent and/or troubleshoot potential future issues.
[ ] Create or edit the function documentation. Include working examples.
[ ] Update or add the new functionality to the appropriate vignette (or create new one).
[ ] If function/code edits made as part of this issue impact other functions in the package or functionality in the shiny app, ensure those are updated as well.
[ ] Run TADA_UpdateAllRefs(), TADA_UpdateExampleData(), styler::style_pkg(), devtools::document(), and devtools::check() and address any new notes or issues before creating a pull request.
[ ] Run more robust check for releases: devtools::check(manual = TRUE, remote = TRUE, incoming = TRUE)