954 create helper function to auto populate the saltfresh column in tada criteria and methodologies template for users - #1010
Conversation
|
Spelling check failed. Details: |
for more information, see https://pre-commit.ci
|
Spelling check failed. Details: |
…sh-column-in-tada-criteria-and-methodologies-template-for-users' of https://github.com/USEPA/EPATADA into 954-create-helper-function-to-auto-populate-the-saltfresh-column-in-tada-criteria-and-methodologies-template-for-users
|
coverage-report
Minimum allowed coverage is Generated by 🐒 cobertura-action against 34e69b5 |
|
if ATTAINS.WaterType hasn't been assigned to a data frame yet, I noticed that if you accidentally made a typo ex I accidentally typed AU' instead of AU, the function will run with the default as ML. Should there be an error if the arg input is not one of AU or ML? I notice also that when using location_col = "ML" the data frame grows a significant amount (ex. Data_Nutrients_UT would result in 6.6M rows). Is this expected? Are there one to many matches occuring in the WQPMonLocTypeToSaltFresh.csv? |
…te-the-saltfresh-column-in-tada-criteria-and-methodologies-template-for-users
Thanks for these questions - will get back to you with a response (and likely some code fixes) later this week. |
|
@wokenny13 -I fixed the join bug that was creating extra rows in the resulting df. |
|
@wokenny13 - I added stops with messages if the user incorrectly enters params. |
…te-the-saltfresh-column-in-tada-criteria-and-methodologies-template-for-users
…te-the-saltfresh-column-in-tada-criteria-and-methodologies-template-for-users
|
Would there be any value/ potential interest from users to provide their own custom reference table as an arg input for a user's own crosswalk table (rather than using the internal TADA tables) that would allow users to review this list, and make any edits as needed? This could potentially be another function that creates a sort of own WQPMonLocTypeToSaltFresh/ATTAINSWaterTypeToSaltFresh ref output for users to only review the unique ATTAINS.WaterType or WQP MonitoringLocationIdentifier and the TADA default of Salt/Fresh from TADA, and then users can customize as desired. Users would not need to go through this step and can use the default TADA internal Salt/Fresh table if desired. But if they do feel the need to customize they can do so. I was thinking this could help with any tricky classification of waters that could be either salt or fresh. |
…te-the-saltfresh-column-in-tada-criteria-and-methodologies-template-for-users
I like this idea! Yes it makes sense to me that users might want to provide their own crosswalk, particularly to deal with tricky water type like estuaries. I will make some updates to:
@wokenny13 and @cristinamullin would your preference be that I do this additional work as part of this PR or a new one? The major pro to merging this first draft in before the update is that it would allow me to start working on adding this tab in the Shiny app. |
|
I think as a separate PR is fine. Other than that suggestion, I think this PR is good to merge in. |
Closes #954
Adds TADA_SaltFreshIndicator which populates the TADA.SaltFreshIndicator column to support mod 2 workflow and provide salt/fresh info for criteria in Mod 3.
This function will be added to an expanded Mod 2 vignette update in the near future.
This PR adds:
Reviewing the reference table I added to crosswalk to the saltfresh indicator by water type as well as the funciton itself would be helpful. I really was not sure what to to with estuaries - should they get their own indicator value?
Pull Request Checklist (convert PR to draft if in progress)
Required
Update your branch from the latest
developand resolve any merge conflictsRun devtools::test(), devtools::check(), and devtools::document() locally; ensure tests pass and fix any errors, warnings, or notes. Add new dependencies to
DESCRIPTIONand document appropriatelyAdd/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 coveredConditional
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 viaMaintenanceScheduled.Ror trigger the Component File Update GitHub ActionIf new example data files were added, document them in
ExampleData.Rand include them inMaintenanceScheduled.Rfor regular refreshIf columns were added/updated, update
RequiredCols.RIf changes affect other package or the shiny app functions, update those impacted functions accordingly