-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add thicknessOf()
#306
Add thicknessOf()
#306
Conversation
Nice idea. I'll do some testing this week, |
R/thicknessOf.R
Outdated
#' # note that the latter includes the thickness of E horizons between the A and the B | ||
#' | ||
#' # when using a custom function (be sure to accept ... and consider the effect of NA values) | ||
#' | ||
#' # calculate cumulative thickness of horizons containing >18% clay | ||
#' thicknessOf(jacobs2000, FUN = function(x, ...) !is.na(x[["clay"]]) & x[["clay"]] > 18) | ||
#' thicknessOf(jacobs2000, prefix = "claygt18_", | ||
#' FUN = function(x, ...) !is.na(x[["clay"]]) & x[["clay"]] > 18) | ||
#' | ||
thicknessOf <- function(x, | ||
pattern = NULL, | ||
hzdesgn = guessHzDesgnName(x, required = TRUE), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't remember if we wanted to default to guessing (across all functions) when horizon names aren't specified or set in metadata. Currently there is a mixture of approaches across aqp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you have more recently decided to not guess (which has broken or changed output in several places), but I don't think there is a rule we agreed on. If you are making one then we can start a change here.
The functions for guessing horizon designation, texture class names, and generic attributes are used in several places (pretty much all "my" functions I suppose) I could replace all uses of guessxx() with the metadata getter, and required=TRUE argument, but for now this is consistent with many of my other function usages. Let's address that (deprecating the 3 guess functions) in a separate issue or PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have started to address this in #309, and will make a corresponding fix in this PR. Thanks for prodding me on this, there was lots of room to improve consistency and I am happy to tackle it across all functions.
Tiny request: can you please add in a couple of comments so that I can keep track of the logic. Thanks. |
Done in a466959 |
Merging this now, can make any adjustments/additional method demonstrations in future vignette or examples |
Adds a commonly sought-after operation
thicknessOf()
allowing for calculation of cumulative or "minmax" thickness of horizons matching some criteriaThis function calculates the cumulative (
method="cumulative"
, default) or maximum difference between (method="minmax"
) horizons within a profile that match a defined pattern (pattern
) or, more generally, any set of horizon-level logical expressions encoded in a function (FUN
).TODO:
returnI actually dont think this is necessarypattern
(likedepthOf()
)horizonDepths(x)
)EDIT: fix for "minmax"