Skip to content

fix(cli): infer delimiters for TSV inputs - #549

Open
AtomicGlance wants to merge 2 commits into
capitalone:mainfrom
AtomicGlance:agent/infer-cli-delimiters
Open

fix(cli): infer delimiters for TSV inputs#549
AtomicGlance wants to merge 2 commits into
capitalone:mainfrom
AtomicGlance:agent/infer-cli-delimiters

Conversation

@AtomicGlance

Copy link
Copy Markdown

Addresses #545.

What changed

  • Infer comma or tab delimiters separately for each input file.
  • Recognize .tsv and .tab as CSV inputs across pandas, Polars, and Spark.
  • Keep --csv-delimiter as an override for both inputs.
  • Replace the old TSV limitation test with same-format and mixed CSV/TSV coverage, including a Spark regression test.
  • Update the CLI documentation to describe the inferred extensions.

Why

The CLI already inferred each file's format independently, but the delimiter was one shared value that defaulted to a comma. A TSV file could therefore reach the correct reader and still be parsed into one malformed column. Keeping the parser default unset lets the loaders distinguish an explicit override from normal extension-based inference.

Validation

  • pytest tests/cli/test_compare.py tests/cli/test_parser.py tests/cli/test_spark.py -q — 124 passed, 5 skipped
  • ruff check datacompy/cli tests/cli
  • ruff format --check datacompy/cli tests/cli
  • mypy --follow-imports=skip --ignore-missing-imports datacompy/cli/backends.py
  • sphinx-build -b html docs/source docs/build/html (builds with the existing api/modules toctree warning)

@CLAassistant

CLAassistant commented Aug 3, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@AtomicGlance
AtomicGlance marked this pull request as ready for review August 3, 2026 22:03
@AtomicGlance
AtomicGlance requested review from a team as code owners August 3, 2026 22:03
@fdosani

fdosani commented Aug 4, 2026

Copy link
Copy Markdown
Member

Thanks for the PR. I'd like to hold off merging this in right now as I want to focus on getting the original PR reviewed and merged. I'll leave it be for now, but maybe get you to do a rebase/alteration once we are ready to move forward with this.

@fdosani
fdosani deleted the branch capitalone:main August 6, 2026 15:24
@fdosani fdosani closed this Aug 6, 2026
@fdosani fdosani reopened this Aug 6, 2026
@fdosani
fdosani changed the base branch from cli-feature to main August 6, 2026 15:24

@fdosani fdosani 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.

Taking another pass at the review. Sorry about the confusion

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.

3 participants