Skip to content

check_data() does not check factor levels fully (297) - #298

Merged
joethorley merged 14 commits into
mainfrom
issue-297
Jul 14, 2026
Merged

check_data() does not check factor levels fully (297)#298
joethorley merged 14 commits into
mainfrom
issue-297

Conversation

@StefanoMezzini

@StefanoMezzini StefanoMezzini commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

fixes #297. changes made:

  • added tests that pass with the previous versions of the functions

@StefanoMezzini

StefanoMezzini commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

all tests passed locally using a copy of chk_all_na() from #299. I will need to merge #299 (or main) into this branch for the tests to pass.

@StefanoMezzini
StefanoMezzini marked this pull request as ready for review July 8, 2026 13:45
claude[bot]
claude Bot reviewed Jul 12, 2026
View reviewed changes

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

Tip: disable this comment in your organization's Code Review settings.

@joethorley @claude
Fix factor edge cases in check_values()
5c63017 
- Empty factor values (factor()) again check class only, as documented
- Remove chk_gt(nlevels(x)) which leaked the internal variable name and
  failed zero-length columns
- Remove dead chk_subset(unique(values), ...) check
- Update details documentation to describe the new factor semantics

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@joethorley joethorley mentioned this pull request Jul 12, 2026
Merged
joethorley and others added 3 commits July 12, 2026 12:22
@joethorley @claude
Drop '(the first occurence of)' from chk_orderset() error message
762dd89 
Breaking change: code matching the previous message text will need
updating.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@joethorley @claude
Restore one-level factor idioms in check_values()
cf62468 
A values factor with less than two levels no longer checks levels:
factor(1) allows any factor without missing values, factor(c(1, NA))
any factor with or without missing values and factor(NA) any factor
with all missing values. Levels must be identical, including order,
only when values has two or more levels.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@joethorley @claude
Cross-reference check_values() details from check_data() documentation
b9e7f08 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
joethorley
joethorley reviewed Jul 12, 2026
View reviewed changes

@joethorley joethorley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See modifications including slight change in behaviour different to discussed

#301

@StefanoMezzini

StefanoMezzini commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

happy to pull #301 once it is complete into this PR

@joethorley @claude @StefanoMezzini
Fix factor edge cases in check_values() (#298 review) (#301)
b201ef3 
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Stefano Mezzini <stefano@poissonconsulting.ca>
@joethorley @claude
Merge branch 'issue-297' into issue-297-review-fixes
9a9a0c2 
Resolves the parse error in test-check-data.R introduced by #301
while keeping the empty-factor test case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
claude[bot]
claude Bot reviewed Jul 14, 2026
View reviewed changes

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

Tip: disable this comment in your organization's Code Review settings.

@joethorley
joethorley merged commit efc8c0e into main Jul 14, 2026
9 checks passed
@joethorley
joethorley deleted the issue-297 branch July 14, 2026 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@claude claude[bot] claude[bot] left review comments
@joethorley joethorley joethorley approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

check_data() does not check factor levels fully

2 participants

@StefanoMezzini @joethorley