Skip to content

fix: warn when strl_threshold is out-of-range instead of silent clipping#821

Open
LeonidasZhak wants to merge 2 commits into
tidyverse:mainfrom
LeonidasZhak:fix/strl-threshold-warn
Open

fix: warn when strl_threshold is out-of-range instead of silent clipping#821
LeonidasZhak wants to merge 2 commits into
tidyverse:mainfrom
LeonidasZhak:fix/strl-threshold-warn

Conversation

@LeonidasZhak

Copy link
Copy Markdown

Summary

validate_strl_threshold() previously silently clipped out-of-range values (< 0 or > 2045) to 2045 without informing the user. This PR adds a cli_warn() so users know their value was adjusted.

Changes

File Change
R/haven-stata.R Add cli_warn() in validate_strl_threshold() when clipping occurs
tests/testthat/test-haven-stata.R Add test for out-of-range warning; update existing test to expect_warning()

Motivation

If a user passes strl_threshold = 5000, they expect strings > 5000 bytes to use strL storage. But the value was silently clipped to 2045 (the max str# length). Now the user gets a warning:

Warning: `strl_threshold` must be between 0 and 2045, not 5000.

Validation

  • devtools::test(filter = "haven-stata"): 70 PASS, 0 new failures
  • devtools::test(): 474 PASS, 2 pre-existing failures (labelled-pillar snapshot, haven-sas deprecation)

Notes

- Add test verifying write_dta() strl_threshold correctly controls strL
  vs str# storage with both low (50) and high (5000) thresholds
- Fix 'udpates' -> 'updates' typo in test-as_factor.R test description
validate_strl_threshold() previously silently clipped out-of-range
values (< 0 or > 2045) to 2045 without informing the user. Now it
emits a cli_warn() so users know their value was adjusted.

- R/haven-stata.R: add cli_warn() in validate_strl_threshold()
- tests/testthat/test-haven-stata.R: add test for out-of-range
  warning behavior; update existing test to expect_warning()
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.

1 participant