149 address depth profile data #386
Conversation
Draft function working other than displaying "adding missing grouping variables" - need to figure out where that is coming from
Documentation and print message updates
Update CharacteristicName to TADA.CharacteristicName in function
updated print messages for daily agg options
updates to documentation and remove intermediate objects
Added clean option for returning data frame. Style updates.
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #386 +/- ##
===========================================
- Coverage 46.21% 40.61% -5.61%
===========================================
Files 14 17 +3
Lines 3410 4656 +1246
===========================================
+ Hits 1576 1891 +315
- Misses 1834 2765 +931 ☔ View full report in Codecov by Sentry. |
R/ResultFlagsIndependent.R
Outdated
| #' day and location. User input also defines whether aggregation occurs over | ||
| #' each depth category or for the entire depth profile. | ||
| #' | ||
| #' @param .data TADA dataframe which must include the columns ADD DEPTH COLUMNS |
R/ResultFlagsIndependent.R
Outdated
| #' When daily_agg == "min" or when daily_agg == "max", the min or max | ||
| #' value in each group of results (as determined by the by category param) will | ||
| #' be determined for each MonitoringLocation, ActivityDate, and TADA.CharacteristicName | ||
| #' combination. An additional column, TADA.ResultValueAggregation.DepthProfile.Flag will be added |
There was a problem hiding this comment.
How about TADA.DepthProfileAggregation.Flag ?
There was a problem hiding this comment.
I like that - will make the change.
R/ResultFlagsIndependent.R
Outdated
| #' combination. An additional column, TADA.ResultValueAggregation.DepthProfile.Flag will be added | ||
| #' to describe aggregation. | ||
| #' | ||
| #' @param bycategory Boolean argument with options "TRUE" or "FALSE". The default is |
There was a problem hiding this comment.
It may be helpful if the user could choose the specific category here (e.g. "Epilimnion-surface", "Hypolimnion-bottom", "Metalimnion/Thermocline-middle", or "All".
The default would be "All".
There was a problem hiding this comment.
Would "all" combine all categories? Or give a separate aggregate value for each category?
R/ResultFlagsIndependent.R
Outdated
| #' any calculated descriptive statistics are determined for each depth category for each | ||
| #' Monitoring Location. | ||
| #' | ||
| #' @param clean Boolean argument with options "TRUE" or "FALSE". The default is |
There was a problem hiding this comment.
"clean" or "aggregatedonly"? aggregatedonly is similar to other flag functions flaggedonly function input. I think it would work better here.
We use clean in other other functions to remove data that is invalid or not applicable to the dataset. In this situation for depth data, clean might be more applicable to use to remove results that do not include depth data and therefore could not be categorized in TADA.DepthCategory.Flag. I wonder if that should be it's own separate function - a flag function for finding results that do not include any depth information. OR if that functionality should be included in this function via the clean parameter.
There was a problem hiding this comment.
Thanks for the explanation. I will change clean to "aggregatedonly" for this function. I think a "clean" option for this function also makes sense.
R/ResultFlagsIndependent.R
Outdated
| dplyr::ungroup() %>% | ||
| # assign depth categories by using depth information | ||
| dplyr::mutate(TADA.DepthCategory.Flag = dplyr::case_when( | ||
| !MonitoringLocationTypeName %in% c("Lake, Reservoir, Impoundment", "Great Lakes", "Lake", "Great Lake") ~ "Not Calculated For MonitoringLocationType", |
There was a problem hiding this comment.
Does this function only work for these location types?
"Lake, Reservoir, Impoundment", "Great Lakes", "Lake", "Great Lake"
Suggest to add details about this to function documentation.
Added depth column info to documentation, changed clean to aggregatedonly, specified location types in documentation
Updated dailyagg and documentation, made sure depthcat.list was used throughout function, updated printed messages to match updated dailyagg
fixed error in example
|
It looks like TADA_ConvertResultUnits() is still not working as expected. There are units in our reference table (WQXunitRef) with different target units. With the current implementation, all length units should be standardized to the same unit within that category. Which I agree which may not be appropriate for every characteristic. To implement characteristic specific units, we started to discuss a new approach but it has not been implemented: #398
|
|
In the current WQX unit ref table, it looks like both m and ft are listed as target units. I think removing one or the other (remove ft?), so that only one length unit is listed as the target would solve the problem in the short term and help fix the issues I'm having with the depth profile functions. Should I give that a try? Or I could work on the new approach to unit conversion and revisit the depth profile functions after. What is your preference? |
|
@cristinamullin I updated this branch to be consistent with develop. I will start working on this again once we've wrapped up the unit conversion work. |
depth profile updates
switch from stringr to base R approach for identifying depth params
stringr --> base R
updated examples so they work
update global variables
spelling updates
commit d0ec317 Author: hillarymarler <Marler.Hillary@epa.gov> Date: Mon May 20 09:17:41 2024 -0400 Added examples for new functions Examples added for TADA_ColorPalette and TADA_ViewColorPalette commit 28307ab Author: hillarymarler <Marler.Hillary@epa.gov> Date: Mon May 20 09:06:04 2024 -0400 Fix for TADA_FieldValuesPie bug Moved ref to TADA color palette to correct place in code commit fcd6e40 Author: hillarymarler <Marler.Hillary@epa.gov> Date: Mon May 20 08:23:39 2024 -0400 Ref correct branch in develop only code chunk commit 114de9b Author: hillarymarler <Marler.Hillary@epa.gov> Date: Mon May 20 08:11:09 2024 -0400 Update branch in vignettes commit 288ecae Merge: 19aa574 8fb62b3 Author: hillarymarler <152432687+hillarymarler@users.noreply.github.com> Date: Fri May 17 13:38:09 2024 -0500 Merge branch 'develop' into colorpalette_set_up commit 19aa574 Author: hillarymarler <Marler.Hillary@epa.gov> Date: Fri May 17 14:31:00 2024 -0400 TADA_OverviewMap Updates Outline for circle markers works now, needed to specify color as character in color arg. commit 0c56afb Author: hillarymarler <Marler.Hillary@epa.gov> Date: Fri May 17 14:23:15 2024 -0400 OverviewMap Updates Changed outline color for circle markers, updated code to display correct color when there is only one bin. commit 81ad9c7 Author: hillarymarler <Marler.Hillary@epa.gov> Date: Fri May 17 14:15:49 2024 -0400 Update OverviewMap Updates to test building new palette based on blues in tada.palette. commit 0131285 Author: hillarymarler <Marler.Hillary@epa.gov> Date: Fri May 17 11:02:27 2024 -0400 Fixed TADA_ViewColorPalette bug Moved calculation of length of palette to earlier in workflow to fix issues commit bb3f0f1 Author: hillarymarler <Marler.Hillary@epa.gov> Date: Fri May 17 10:52:05 2024 -0400 Pie chart colors update Switched colors for pie chart to new palette for consistency commit 8fb62b3 Author: cristinamullin <46969696+cristinamullin@users.noreply.github.com> Date: Thu May 16 16:39:44 2024 -0400 Update tribal shapefiles, WQXCharacteristicRef.csv and WQXcharValRef.csv commit 4b39e4a Author: hillarymarler <Marler.Hillary@epa.gov> Date: Thu May 16 16:27:10 2024 -0400 Wrote code for tada.blue palette Wrote code to create a tada palette of blues, mostly for use in maps. It is currently located in the overview map function. May be useful to make it its own function? commit b4f21fd Author: hillarymarler <Marler.Hillary@epa.gov> Date: Thu May 16 15:31:08 2024 -0400 Update boxplot colors Called the light and dark blue colors directly from the color palette commit 65be182 Author: hillarymarler <Marler.Hillary@epa.gov> Date: Thu May 16 15:26:37 2024 -0400 Added examples for palette functions Added simple examples for palette functions, removed label_space variable (not needed) commit aba6957 Author: hillarymarler <Marler.Hillary@epa.gov> Date: Thu May 16 15:18:26 2024 -0400 First draft of TADA_ColorPalette and TADA_ViewColorPalette Functions to create and view a color swatch of the color palette commit adb45dd Merge: fa5034d 73923c8 Author: Cristina Mullin <46969696+cristinamullin@users.noreply.github.com> Date: Tue May 14 10:49:04 2024 -0400 Merge pull request #461 from USEPA/markdown_demo Update TADAAssessmentUnitUseCase.Rmd commit 73923c8 Author: hillarymarler <Marler.Hillary@epa.gov> Date: Tue May 14 09:12:41 2024 -0400 Update TADAAssessmentUnitUseCase.Rmd updates to table format
Fixed issues with depth param as 2nd param, added hover text for depth cat to show depth, created lighten color function to create line colors based on the marker colors used, and updated function to use tada.pal colors
subsetting data based on user input, started work on figure (not working yet, strange things happening when lines are added)
Removed unit conversion and new function draft (will finish in later pull request), minor updates to documentation.
Added imports from grDevices and graphics
Updated wordlist and global variables based on suggestions from check()
Function to identify depth categories and determine mean, max, or min (defined by user input) by either monitoring location/date or monitoring location/date/depth category.
Should there be a "clean" option which would return only the aggregate values for groups with n > 1?