Skip to content

Follow-up on feedback from CAA #55

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.

Sign up for GitHub

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

Draft
archaeothommy wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Follow-up on feedback from CAA #55

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6a0a55c
emphasize in documentation that read_ASTR() call data to ASTR objects
Jul 4, 2026
5ffe7b1
first function draft
Jul 4, 2026
a91aa86
first draft of reworked bdl handling
nevrome Jul 10, 2026
d628e49
update negative bdl strategy with regulat expression
Jul 11, 2026
5517f9f
include markers and replacement values in function parameters, update…
Jul 11, 2026
2a7edf6
replace default value [skip ci]
Jul 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NAMESPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ export(abs_to_rel)
export(albarede_juteau_1984)
export(as_ASTR)
export(at_to_wt)
export(bdl_strategy_default)
export(bdl_strategy_negative)
export(bdl_strategy_none)
export(copper_alloy_bb)
export(copper_alloy_pollard)
export(copper_group_bray)
Expand Down
41 changes: 16 additions & 25 deletions R/ASTR_basic.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
#' \itemize{
#' \item **as_ASTR**: Transforms an R `data.frame` to an object of class
#' `ASTR`.
#' \item **read_ASTR**: Reads data from a file (.csv, .xls, .xlsx) into
#' an object of class `ASTR`.
#' \item **read_ASTR**: Reads data from a file (.csv, .xls, .xlsx) and
#' converts it into an object of class `ASTR`.
#' \item **validate**: Performs additional validation on `ASTR` and returns
#' a `data.frame` as a workable list of potential issues.
#' \item **get_..._columns**: Subsets `ASTR` tables to columns of a certain
Expand All @@ -33,10 +33,10 @@
#' @param id_column name of the ID column. Defaults to "ID"
#' @param context columns that provide contextual (non-measurement) information;
#' may be column names, integer positions, or a logical inclusion vector
#' @param bdl strings representing “below detection limit” values. By default,
#' the following are recognized: "b.d.", "bd", "b.d.l.", "bdl", "<LOD", "<"
#' @param bdl_strategy function used to replace BDL strings. Defaults to a
#' static function returning `NA`
#' @param bdl_strategy function used to replace "below detection limit" strings.
#' See [bdl_strategies] for the different available strategies and on how to
#' implement a custom one. Use [purrr::compose()] to combine different
#' [bdl_strategies] (see examples).
#' @param guess_context_type should appropriate data types for contextual
#' columns be guessed automatically? Defaults to `TRUE`
#' @param na character vector of strings to be interpret as missing values. By
Expand Down Expand Up @@ -66,18 +66,10 @@
#' unless `drop_columns = TRUE` (then it will result in warnings for the
#' respective columns).
#'
#' Below detection limit notation (i.e. ‘b.d.’, ‘bd’, ‘b.d.l.’, ‘bdl’, ‘<LOD’,
#' or ‘<..’) for element and oxide concentrations is specified using the `bdl`
#' argument. One or more notations can be used as is appropriate for the
#' dataset, and can be notations not included in the list above. The argument
#' `bdl_strategy` is used to specify the value for handling detection limits.
#' This is to facilitate the different handling needs of the detection limit
#' for future statistical applications, as opposed to automatically assigning
#' such values as ‘NA’.
#'
#' Missing values are allowed anywhere in the data file body, and will be
#' replaced by `NA` automatically.
#'
#'
#' @examples
#' library(magrittr)
#'
Expand All @@ -92,6 +84,11 @@
#' # validating an ASTR table
#' validate(arch)
#'
#' # combining bdl strategies
#' arch2 <- as_ASTR(test_df, id_column = "Sample", context = 1:7,
#' bdl_strategy = purrr::compose(bdl_strategy_default, bdl_strategy_negative)
#' )
#'
#' # extracting subsets of columns
#' conc <- get_concentration_columns(arch) # see also other get_..._columns functions
#'
Expand Down Expand Up @@ -120,12 +117,7 @@
#' @export
as_ASTR <- function(
df, id_column = "ID", context = c(),
bdl = c("b.d.", "bd", "b.d.l.", "bdl", "<LOD", "<"),
bdl_strategy = function() NA_character_,
# this only allows static functions, essentially: bdl_replace = "NA"
# in case more sophisticated handling is desired:
# bdl_strategy = function(x, colname) { bdl_lookup_table[colname] / sqrt(2) }
# bdl_lookup_table = c("Fe_%" = 3)
bdl_strategy = bdl_strategy_default,
guess_context_type = TRUE,
na = c(
"", "n/a", "NA", "N.A.", "N/A", "na", "-", "n.d.", "n.a.",
Expand Down Expand Up @@ -166,7 +158,7 @@ as_ASTR <- function(
dplyr::ungroup()
# determine and apply column types
column_table <- parse_colnames(df2, context, drop_columns)
constructors <- build_constructors(column_table, bdl, bdl_strategy, guess_context_type, na)
constructors <- build_constructors(column_table, bdl_strategy, guess_context_type, na)
col_list <- purrr::map2(df2, constructors, function(col, f) f(col)) %>%
purrr::discard(is.null)
df3 <- as.data.frame(col_list, check.names = FALSE)
Expand Down Expand Up @@ -222,8 +214,7 @@ read_ASTR <- function(
"", "n/a", "NA", "N.A.", "N/A", "na", "-", "n.d.", "n.a.",
"#DIV/0!", "#VALUE!", "#REF!", "#NAME?", "#NUM!", "#N/A", "#NULL!"
),
bdl = c("b.d.", "bd", "b.d.l.", "bdl", "<LOD", "<"),
bdl_strategy = function() NA_character_,
bdl_strategy = bdl_strategy_default,
drop_columns = FALSE,
validate = TRUE,
...
Expand Down Expand Up @@ -284,7 +275,7 @@ read_ASTR <- function(
as_ASTR(
input_file,
id_column = id_column, context = context,
bdl = bdl, bdl_strategy = bdl_strategy,
bdl_strategy = bdl_strategy,
guess_context_type = guess_context_type, na = na,
drop_columns = drop_columns,
validate = validate
Expand Down
40 changes: 40 additions & 0 deletions R/ASTR_bdl_strategies.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#' @name bdl_strategies
#'
#' @title Strategies to replace "below detection limit" values
#'
#' @description ...
#'
#' @param x a vector, derived from a data.frame column
#' @param colname name of the respective data.frame column
#' @param marker values indicating "below detection limit"
#' @param value value to replace values identified as "below detection limit"
#' @param ... further arguments passed to or from other methods
#'
#' @rdname bdl_strategies
#'
#' @examples
#' plot(1,1)
#'
NULL

#' @rdname bdl_strategies
#' @export
bdl_strategy_default <- function(x, colname, marker = c("b.d.", "bd", "b.d.l.", "bdl", "<LOD", "<"), ...) {
bdl_indices <- which(grepl(paste(marker, collapse = "|"), x, perl = FALSE))

@nevrome nevrome Jul 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These bdl strategy functions can not have additional arguments just so. They are not called by the user (bdl_strategy_default()), but forwarded by the user as a higher order function to as_ASTR (bdl_strategy_default). That means they can only have the arguments expected by as_ASTR when it calls it through build_constructors():

bdl_strategy(x = x, colname = colname)

Think of it in terms of types: as_ASTR takes a bdl_strategy argument, which is a function of type

bdl_strategy :: Vector a -> String -> Vector a

If you now give it a function of type

bdl_strategy :: Vector a -> String -> Vector String -> Vector a

as you do with function(x, colname, marker = c("b.d.", "bd", "b.d.l.", "bdl", "<LOD", "<"), ...) then this is just inconsitent. If R had a static type system this would not compile.

Probably this still works here in the extremely flexible R, because marker has a default value here. But users can not change marker as things stand, and then it's misleading to expose the argument.

If you want the user to be able to change marker then you have three options:

  1. Add an argument bdl_marker to read_ASTR and as_ASTR and forward it to the bdl_strategy through build_constructors. This is pretty tedious if you want to cover the arguments of multiple different strategy functions.
  2. Make a function factory that generates self-contained bdl strategy functions. This could be a function of type
make_keyword_filter_bdl_strategy :: Vector String -> (Vector a -> String -> Vector a)

It takes marker and returns a function that has the markers encoded in its body. This is then exactly what as_ASTR expects and can handle.

  1. Ask users to write their own bdl strategy functions if they need anything beyond the default ones. Then they are the function factory. As these functions are so little and clear this it not too much to ask, imho. Realistically: How often would users encounter this anyway?

x[bdl_indices] <- NA_character_
return(x)
}

#' @rdname bdl_strategies
#' @export
bdl_strategy_none <- function(x, colname, ...) {
return(x)
}

#' @rdname bdl_strategies
#' @export
bdl_strategy_negative <- function(x, colname, value = "99999", ...) {
bdl_indices <- which(grepl("^-\\d*\\.?\\d*\\*?\\d*\\^?\\-?\\d*$", x))

@nevrome nevrome Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that the bdl strategy functions are called by as_ASTR, which accepts any data.frame. The assumption that the column would certainly be of type character is wrong, if I'm not missing anything. This should work in both cases:

y <- suppressWarnings(as.numeric(x))
bdl_indices <- which(y < 0)

Sorry for missing this initially. I was too focused on the read_ASTR case.

@archaeothommy archaeothommy Jul 20, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will revert the code to your generalised version then. Thanks!

x[bdl_indices] <- value
return(x)
}
12 changes: 2 additions & 10 deletions R/ASTR_colname_parser.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ parse_colnames <- function(x, context, drop_columns) {
# 2. build constructor functions
build_constructors <- function(
column_table,
bdl, bdl_strategy,
bdl_strategy,
guess_context_type, na
) {
purrr::pmap(
Expand All @@ -224,7 +224,7 @@ build_constructors <- function(
function(x) {
# bdl
if (consider_bdl) {
x <- apply_bdl_strategy(x, colname, bdl, bdl_strategy)
x <- bdl_strategy(x = x, colname = colname)
}
# type
if (type == "numeric") {
Expand Down Expand Up @@ -268,14 +268,6 @@ as_numeric_info <- function(x, colname) {
return(y)
}

# colname only an argument in case we want to implement more specific handling
# eventually
apply_bdl_strategy <- function(x, colname, bdl, bdl_strategy) {
bdl_values <- which(grepl(paste(bdl, collapse = "|"), x, perl = FALSE))
x[bdl_values] <- bdl_strategy()
return(x)
}

#### regex validators ####

is_err_percent <- function(colname) {
Expand Down
33 changes: 13 additions & 20 deletions man/ASTR.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 38 additions & 0 deletions man/bdl_strategies.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vignettes/ASTR.showcase.Rmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ data

Note how the column headers are organised according to the ASTR conventions. This allows the package to _understand_ some of the semantics of the dataset in the `read_ASTR()` process, and correctly represent them in the resulting `ASTR` object. A closer look at the mock-up data also reveals (common) problems with the copper isotope data: it seems that some Excel formulas did not work as intended and left `#REF!` entries. `read_ASTR()` will read these as `NA` values. Finally, every `ASTR` object requires one column that acts as a unique row identifier. In this dataset there is no such column, though, only a `Group` identifier. `read_ASTR()` can automatically turn such a column to a unique identifier.

We can now perform the reading process. As we are working with mock-up data in an R vignette, we do not read from the file system. We only need to call an essential internal function of `read_ASTR()`: `as_ASTR()`. It turns R data.frames to `ASTR` objects. If we would start from an Excel file we would call `read_ASTR()` instead.
We can now perform the reading process. As we are working with mock-up data in an R vignette, we do not read from the file system. We only need to call an essential internal function of `read_ASTR()`: `as_ASTR()`. It turns R data.frames to `ASTR` objects. If we would start from an Excel file we would call `read_ASTR()` instead, which internally calls `as_ASTR()` to convert the data into an ASTR object after reading the file.

To do this in practice, we not only have to submit our `data` to `as_ASTR()`, but also set two other arguments: 1. We have to define one of the columns as the ID column with `id_column`, and 2. we explicitly have to mark any column providing contextual information with `context` (i.e. no analytical values, cf. [ASTR schema: Implementation](VG.ASTR.Schema.Implementation.html)).

Expand Down
Loading