Skip to content

Add POC helpers for NSE resolution and where subsetting#191

Merged
dbosak01 merged 2 commits intodbosak01:masterfrom
ankonyeni:poc/nse-where-helpers
Mar 18, 2026
Merged

Add POC helpers for NSE resolution and where subsetting#191
dbosak01 merged 2 commits intodbosak01:masterfrom
ankonyeni:poc/nse-where-helpers

Conversation

@ankonyeni
Copy link
Copy Markdown
Collaborator

This PR is a proof of concept only and is not intended to be merged.

The goal is to get feedback on whether it makes sense to centralize two repeated internal patterns that appear across the package:

  1. NSE handling for arguments that support unquoted column names
  2. data subsetting through the where argument

For this POC, I added two internal helpers in R/utilities.R:

  • resolve_arg()
  • subset_data()

I then updated the current repeated call sites in the affected procedures and plot helpers to use those functions.

What changed

resolve_arg()

resolve_arg() is meant to replace the repeated pattern that currently does:

argo <- deparse(substitute(arg, env = environment()))
arg <- tryCatch(
  {if (typeof(arg) %in% c("character", "NULL")) arg else argo},
  error = function(cond) {argo}
)

The helper:

  • captures the unevaluated argument expression
  • returns the evaluated value when its type is allowed
  • otherwise returns the deparsed expression as a string
  • validates the type argument used by the helper

subset_data()

subset_data() centralizes where filtering and adds explicit validation.

The helper:

  • requires data to be a data.frame
  • returns the input unchanged when where is NULL
  • requires where to be an expression of length 1
  • evaluates where in the data environment
  • requires the result to be a logical vector with length equal to nrow(data)

Files touched

Helpers:

  • R/utilities.R

Updated resolve_arg() call sites:

  • R/proc_freq.R
  • R/proc_means.R
  • R/proc_reg.R
  • R/proc_sort.R
  • R/proc_transpose.R
  • R/proc_ttest.R
  • R/freqplots.R
  • R/regplots.R
  • R/ttestplots.R

Updated subset_data() call sites:

  • R/proc_freq.R
  • R/proc_means.R
  • R/proc_reg.R
  • R/proc_sort.R
  • R/proc_transpose.R
  • R/proc_ttest.R

Tests:

  • tests/testthat/test-utilities.R

Main review points

The most useful feedback for this POC would be:

  1. whether the helper abstraction is worth keeping
  2. whether resolve_arg() preserves current quoted/unquoted behavior
  3. whether parameter validation added in subset_data() works as expected
  4. whether any of the touched functions now behave differently in ways that should block this direction

Please test:

  • quoted vs unquoted column names
  • unresolved bare names
  • numeric argument cases where double or integer is now allowed
  • valid where = expression(...) flows
  • invalid where inputs and the resulting error messages

We also need to consider the placement of the existing zero-row guard in functions that support subsetting through where, since applying the subset may itself produce zero rows:

if (nrow(data) == 0) {
  stop("Input data has no rows.")
}

If this check stays before where processing, it only validates the original input. If it needs to protect downstream logic after subsetting as well, it may need to be revisited in the affected procedures.

Validation so far

I added direct unit coverage for both helpers in tests/testthat/test-utilities.R.

How to pull this branch locally for testing

These commands do not need to be run from the exact same local repo that I used.
They do need to be run from a local Git clone of the procs repository family, for example:

  • your local clone of dbosak01/procs
  • your local clone of your own fork of procs

In other words, the commands should be run from the root of your own local procs repo, not from an unrelated directory.

  • fork: https://github.com/ankonyeni/procs.git
  • branch: poc/nse-where-helpers

Option 1: add my fork as a temporary remote

git remote add ankonyeni https://github.com/ankonyeni/procs.git
git fetch ankonyeni poc/nse-where-helpers
git switch -c test/nse-where-helpers --track ankonyeni/poc/nse-where-helpers

Option 2: one-off fetch without adding a remote

git fetch https://github.com/ankonyeni/procs.git poc/nse-where-helpers:test/nse-where-helpers
git switch test/nse-where-helpers

Cleanup after testing

If you want to remove the local testing branch afterward:

git switch master
git branch -D test/nse-where-helpers

If you added my fork as a temporary remote, you can also remove it:

git remote remove ankonyeni

@ankonyeni ankonyeni requested a review from dbosak01 March 12, 2026 23:33
@ankonyeni ankonyeni self-assigned this Mar 12, 2026
@dbosak01
Copy link
Copy Markdown
Owner

Wow! Looks great! Will review tomorrow.

@dbosak01 dbosak01 marked this pull request as ready for review March 14, 2026 03:49
@dbosak01
Copy link
Copy Markdown
Owner

I like it, and am inclined to just merge it. Do you feel there are any risks? I can go back over some of the testing/validation this weekend. I'm going to do it anyway because I made a few changes this week.

Copy link
Copy Markdown
Owner

@dbosak01 dbosak01 left a comment

Choose a reason for hiding this comment

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

Looks great!

@dbosak01 dbosak01 merged commit 7116f51 into dbosak01:master Mar 18, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants