feat: add Choice field validator for categorical columns - #542
Conversation
|
All CI checks passing. Ready for review! Fixes #194" |
im-anishraj
left a comment
There was a problem hiding this comment.
Thanks @suhavani23 for the clean first pass on Choice. I rechecked the branch against current main; this needs another update before it can be merged.
Blockers:
- The branch now conflicts with current
mainintests/test_schema.pyafter the latest schema work was merged. upstream_tests.txtappears to be an accidental binary/workspace artifact and should be removed from the PR.- Please keep the tests focused and remove unused fixtures/imports where possible; for example,
test_choice_valid_valuestakessample_csvbut then creates its own DataFrame.
Please rebase on latest main, remove the stray file, resolve the schema test conflict by preserving all existing tests, and keep the Choice additions additive.
im-anishraj
left a comment
There was a problem hiding this comment.
Thanks for rebasing, but this head still cannot be approved.
Blockers:
- CI is failing across lint, smoke tests, notebook smoke, and the Python matrix.
upstream_tests.txtis still included as a stray workspace artifact and should be removed from the PR.- The
Choicehelper appears inserted in the middle of theCountryCodesection inarnio/schema.py, leaving the surrounding docstring/body malformed. Please placeChoiceas a standalone helper between complete function definitions. tests/test_schema.pyhas an unrelatedassert result.bad_rows == [1, 3]added insidetest_schema_json_roundtrip_preserves_fields_and_options, whereresultis not part of that test. Please remove that unrelated assertion and keep the new tests additive/focused.test_choice_valid_valuesstill takessample_csvand reads it before replacingframewith a pandas-created frame; please remove the unused fixture/read.
Please remove the artifact, fix the malformed schema/test edits, run Black/Ruff, and make sure CI is green before requesting another review.
|
Hi @suhavani23, maintainer deadline update: please push the requested fixes/rebase or reply with your ETA by May 31, 2026 at 11:59 PM IST. We are not closing this PR right now. This deadline is only to keep the active review queue organized and confirm which PRs are still being worked on. If this PR overlaps with already merged work or another open PR, please also clarify what unique change remains. Thanks. |
|
Hey @im-anishraj , acknowledged! I'll have the fixes pushed by June 5. Thanks for the patience! |
5f12497 to
50b06c4
Compare
|
@suhavani23 is attempting to deploy a commit to the xtylishanish-gmailcom's projects Team on Vercel. A member of the Team first needs to authorize it. |
50b06c4 to
b439c71
Compare
|
Hi @suhavani23, thanks for pushing the update. I checked the latest head (�439c71) and this still needs fixes before it can be re-reviewed for merge.\n\nCurrent blockers from CI:\n\n- Black still fails and would reformat �rnio/frame.py, �rnio/schema.py, and ests/test_frame.py.\n- The Python test matrix fails on ests/test_website_api_docs.py::test_website_api_reference_mentions_public_exports because Choice is now exported publicly but is not documented in the website API reference.\n\nPlease run Black on the changed files and add the new Choice helper to the public API docs/website reference so the export coverage test passes. I also added status:blocked while these CI failures remain. Vercel is still an external authorization issue, but the lint/test failures above are contributor-side blockers. |
Adds test coverage for ar.Choice() covering valid/invalid values, empty allowed set, bare string rejection, nullable/unique/severity interactions, and row index reporting. Adds README usage example and website API reference entry for Choice(), fixing the website public-exports doc check that started failing once Choice became a documented public export. Fixes im-anishraj#194
|
Hi @im-anishraj — thanks for the review! I've addressed both blockers: Ran Black on frame.py, schema.py, and test_frame.py — all reformatted and CI lint now passes All 48 checks are green except Vercel, which looks like it needs deploy authorization on your end. Ready for another look whenever you get a chance! |
ab034ee
into
im-anishraj:arnio-old-version
Summary
Adds a
Choice()field helper toarnio.schemathat validates categorical columns against an explicit set of allowed values. Builds on the existingallowedvalidation logic in_validate_column. RaisesValueErrorif the allowed set is empty.Linked issue
Fixes #194
Type of change
Area
Testing
make testtests/test_schema.pycovering valid choices, invalid choices, empty allowed set, nullable behavior, and different input types (list, tuple, set).Contributor checklist
feat:,fix:,docs:,test:,refactor:,chore:).