Update DepthProfile.R by hillarymarler · Pull Request #923 · USEPA/EPATADA

hillarymarler · 2026-04-27T18:55:23Z

Pull Request Checklist (convert PR to draft if in progress)

Bug fix for plot error if secchi depth is first characteristic selected. There was an erroneous reference to the 3rd param in the code chunk that build the scatterplot trace for the 1st param. This is now corrected. This should not require any changes in the Shiny app, but should allow the depth profile to work correctly even when secchi is selected as the first param.

Required

Update your branch from the latest develop and resolve any merge conflicts
Run devtools::test(), devtools::check(), and devtools::document() locally; ensure tests pass and fix any errors, warnings, or notes. Add new dependencies to DESCRIPTION and document appropriately
Add/update vignettes for corresponding changes in functionality, list these under articles in _pkgdown.yml, and ensure added/updated vignettes run and build with proper formatting locally
Request review from at least one developer team member (convert PR to ready for review if it was designated as in progress)

Best practices

Include a summary of the changes made and relevant context/motivation
Link issues to auto-close on merge (use Development sidebar or include "Closes #" in the PR)
Refresh inline/block comments for clarity
Update roxygen docs and include examples; review help pages
Add/update tests in tests/testthat; review the bot's coverage report from test-coverage and confirm all changes are covered

Conditional

If there is a bot spelling comment, run spelling::spell_check_package() locally and fix any misspellings; add approved project terms to WORDLIST with spelling::update_wordlist()
If tests fail suggesting internal reference files need a refresh, run .TADA_UpdateRefFiles() and .TADA_UpdateExampleData() locally via MaintenanceScheduled.R or trigger the Component File Update GitHub Action
If new example data files were added, document them in ExampleData.R and include them in MaintenanceScheduled.R for regular refresh
If columns were added/updated, update RequiredCols.R
If changes affect other package or the shiny app functions, update those impacted functions accordingly

github-actions · 2026-04-27T19:21:09Z

coverage-report

File	Coverage		Missing
All files	`53%`	✅
R/DepthProfile.R	`0%`	❌	`103-1964`
R/Utilities.R	`65%`	✅	`513` `631-632` `636` `641-643` `738` `750-779` `867-878` `1022-1024` `1092-1093` `1147-1151` `1261-1262` `1266-1267` `1277-1281` `1286-1291` `1333-1458` `1599` `1666` `1672-1692` `1718-1719` `1728-1742` `1766-1769` `1833-1834` `1851-1853` `1858` `1861-1863` `1917-1920` `1947-2264` `2291-2295` `2304-2308` `2525` `2578` `2584-2587` `2591` `2602` `2621-2622` `2624-2639` `2641` `2643` `2649` `2652-2654` `2656` `2661` `2676` `2682` `2690` `2695` `2703-2708` `2737` `2747-2749`

Minimum allowed coverage is 20%

Generated by 🐒 cobertura-action against 8267739

hillarymarler · 2026-04-27T19:59:12Z

Any thoughts on this error @cefergus or @cristinamullin?

cristinamullin · 2026-04-27T20:08:53Z

Any thoughts on this error @cefergus or @cristinamullin?

This is an error the USGS dataRetrieval R package puts out when the services are not working properly. We can add dontrun to this example.

for more information, see https://pre-commit.ci

surfacevalue or bottomvalue as NULL or "null" are treated as NA; only the categories that can be determined are assigned. If both are NA, the function won’t assign Surface/Middle/Bottom by thresholds but can still fall back to ActivityRelativeDepthName via ARD ref; otherwise “No depth info” is used where appropriate. Ensures depth units are single/consistent and errors otherwise (so you fail fast if upstream unit conversion was missed).

for more information, see https://pre-commit.ci

Behavior after the change dailyagg = "min" or "max": aggregatedonly = FALSE: All original rows are preserved for each group; the selected row is flagged as “Selected as min/max aggregate value …”, and all other rows in the same group are flagged as “Considered in minimum/maximum aggregation … but not selected.” aggregatedonly = TRUE: Only the selected rows are returned (no new IDs, no duplicates). dailyagg = "avg": Remains unchanged (still creates a separate aggregate row). If you want this to also avoid creating a new row, let me know and I’ll provide a parallel patch.

for more information, see https://pre-commit.ci

Unique depth counts: fully addressed (you replaced all occurrences with dplyr::n_distinct(..., na.rm = TRUE)). -Inf bottom guard: addressed via has_depths/case_when (once the pipe above is fixed). Optional: You can safely remove length.units, which is unused.

…PATADA into depth_profile_secchi_bug

for more information, see https://pre-commit.ci

This change ensures that if only some required columns are present, the code reliably falls into the else branch to add the missing columns by calling TADA_FlagDepthCategory.

for more information, see https://pre-commit.ci

This change prevents if(NA) errors by removing all string comparisons to "null" and using is.na(...) to branch on missing thresholds.

Only the unit-conversion branching is changed from a nested if (which can never execute) to an if/else so the conversion actually runs when units don’t match. Everything else remains the same.

…PATADA into depth_profile_secchi_bug

Column‑presence check should use all(...) with an else branch

Keep bind_rows and rm inside the “depth params present” block (brace fix)

for more information, see https://pre-commit.ci

TADA_DepthProfilePlot: fix critical runtime bugs and unreachable logic Add fail-fast argument validation for location, activity_date, and groups; remove unreachable code and verify values exist in the input data. Treat ActivityStartDateTime as optional: auto-create the column if missing and expand required columns to include fields used in hover text. Restrict depth-unit consistency check to non–depth-parameter rows, allowing later conversion of depth-parameter (e.g., Secchi) rows; stop if non-depth units don’t match the requested unit. Ensure profile.data is always defined by using depthprofile.avail when no requested groups are depth-parameter characteristics. Use is.na instead of is.null for surfacevalue and bottomvalue in depth category annotation logic to correctly handle “null” inputs normalized to NA.

for more information, see https://pre-commit.ci

github-actions · 2026-04-28T20:24:02Z