Skip to content
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

Recycle brief if validation expands into multiple steps #564

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

Conversation

yjunechoe
Copy link
Collaborator

@yjunechoe yjunechoe commented Aug 19, 2024

This PR aims to fix #478 by recycling brief when possible.

# Single string `brief` recycles
create_agent(small_table) |> 
  col_exists(c("a", "b"), brief = "one") |> 
  el("validation_set") |> 
  el("brief")
#> [1] "one" "one"

# Multi-length `brief` applies if exactly matched in number of steps produced
create_agent(small_table) |> 
  col_exists(c("a", "b"), brief = c("one", "two")) |> 
  el("validation_set") |> 
  el("brief")
#> [1] "one" "two"

# Errors if not matched in length (columns x segments produces 6 steps here)
create_agent(small_table) |> 
  col_vals_equal(
    c("a", "b"), 0,
    segments = vars(f),
    brief = c("one", "two")
  )
#> Error in `resolve_briefs()`:
#> ! `brief` must be length 1 or 6, not 2

This is a draft PR until I:

  • Figure out the right level of abstraction for the resolve_briefs() helper - namely, should it stand on its own (as current) or wrap over the default autobrief behavior, to host all brief-related logic? (will move forward with a unified resolve_briefs() function)
  • Do the necessary copy-pastes for all user-facing validation functions
  • Write (batch?) tests

Out of scope but worth considering for afterwards is whether we should support glue sytnax here as well.

@yjunechoe yjunechoe marked this pull request as draft August 19, 2024 14:22
@rich-iannone
Copy link
Member

@yjunechoe just a note for future work on this PR: #579 changed settings in .Rproj to strip trailing whitespace. Merging from main and re-saving these files should fix the merge conflict.

@yjunechoe
Copy link
Collaborator Author

Got it - thanks for the heads up!

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.

For steps applied several times, personalized brief is only accessible for the first column
2 participants