feat: add datacompy compare CLI with multi-backend support - #534
feat: add datacompy compare CLI with multi-backend support#534fdosani wants to merge 50 commits into
Conversation
…s and improve error handling for missing Snowflake config
… add comprehensive unit tests
…case ignoring in comparison
…case ignoring in comparison
d0cb953 to
da92835
Compare
rhaffar
left a comment
rhaffar
left a comment
There was a problem hiding this comment.
Really nice feature Faisal - a few comments on my part.
This would be a bigger change than the ones left in my review, but any thoughts on making compare args as passthroughs that we don't explicitly define for the CLI? I like defining the args explicitly, but I'm thinking this will have to mean that any changes to the compare interfaces will have to be reflected here from now on as well. We'd probably still need to keep a few explicit args here for special cases if we were to consider that though (like the spark name and SF config stuff).
| if args.on_index: | ||
| return PandasCompare( | ||
| df1, | ||
| df2, | ||
| on_index=True, | ||
| abs_tol=args.abs_tol, | ||
| rel_tol=args.rel_tol, | ||
| df1_name=args.df1_name, | ||
| df2_name=args.df2_name, | ||
| ignore_spaces=args.ignore_spaces, | ||
| ignore_case=args.ignore_case, | ||
| cast_column_names_lower=args.cast_column_names_lower, | ||
| ) | ||
| return PandasCompare( | ||
| df1, | ||
| df2, | ||
| join_columns=args.on, | ||
| abs_tol=args.abs_tol, | ||
| rel_tol=args.rel_tol, | ||
| df1_name=args.df1_name, | ||
| df2_name=args.df2_name, | ||
| ignore_spaces=args.ignore_spaces, | ||
| ignore_case=args.ignore_case, | ||
| cast_column_names_lower=args.cast_column_names_lower, | ||
| ) |
There was a problem hiding this comment.
We could probably keep this to a single return call by making on_index false if args.on isn't set.
There was a problem hiding this comment.
Good call. I'll build the shared kwargs once and only add the join key that applies, so there's a single return PandasCompare(...):
| if args.on_index and args.on: | ||
| raise BadArgsError("--on and --on-index are mutually exclusive.") | ||
| if not args.on_index and not args.on: | ||
| raise BadArgsError( | ||
| "--on is required (or --on-index for the pandas backend). " | ||
| "Specify at least one join column with --on COL." | ||
| ) |
There was a problem hiding this comment.
Right now we do this check in the Pandas Comparator - if we just pass on_index and join_columns directly to the comparator, I think we could get rid of these checks.
There was a problem hiding this comment.
Fair observation that the rules overlap with PandasCompare. I'd lean toward keeping them at the CLI layer though, for a couple of reasons: the comparator raises a plain ValueError, which isn't a CLIError, so main.py would surface it as a raw traceback instead of a friendly message + exit code 2; and the CLI message references the actual flags (--on / --on-index) rather than the library kwarg names. The --on-index is only supported with --backend pandas check is also CLI-only. There's a small redundancy, but it buys clean CLI errors across all four backends (polars/spark get args.on or [], so they wouldn't hit the comparator guard at all). Happy to discuss further though.
| return _stage_file_to_snowflake(session, ref, fmt, csv_delimiter) | ||
|
|
||
|
|
||
| def _stage_file_to_snowflake( |
There was a problem hiding this comment.
I think the use case for comparing local files is very limited for Snowflake - that said Snowpark has a set of file readers that can be used to read files from a Snowflake internal stage. We could use that to read in Snowpark dataframes that we can pass directly to the Snowflake compare object?
There was a problem hiding this comment.
Agreed the local-file → Snowflake path is a narrow use case. On the Snowpark file readers: they're a nice fit when the data already lives on an internal/external stage, and since SnowflakeCompare accepts a Snowpark DataFrame directly we could pass one straight through. The wrinkle is that those readers read from a stage, not a local path, so for a genuinely local file we'd still need a PUT to a stage first, it relocates the upload step rather than removing it, so it's not a clear simplification over write_pandas. Two options: (a) keep write_pandas for local files and additionally accept @stage/... refs via the Snowpark reader, or (b) drop local-file support for Snowflake entirely and only accept table/stage refs. I lean toward (b) for the first release given how narrow the use case is, what's your preference?
| keys.add_argument( | ||
| "--on", | ||
| action="append", | ||
| dest="on", | ||
| default=None, | ||
| metavar="COL", | ||
| help="Join column name (required unless --on-index is used). Repeat for composite keys: --on id --on date.", | ||
| ) |
There was a problem hiding this comment.
Moreso on the side of personal preference, but could we have this just accept either a single or comma separated list of args?
There was a problem hiding this comment.
Sure, easy to support both. I'll keep action="append" and add a type= callable that splits each value on commas, then flatten, so --on id,date and --on id --on date (and a mix) all work. Only edge case is a column name that literally contains a comma, repeating --on stays as the escape hatch for that, and I'll note it in the help text.
…SONL/NDJSON formats
…and improve error handling
feat: reject local files for Snowflake backend, support comma-separated --on, and tidy compare factories
…SONL/NDJSON formats
…and improve error handling
feat: reject local files for Snowflake backend, support comma-separated --on, and tidy compare factories
|
Dosani, Faisal seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Closing in favour of #544 |
4 participants
Implements the
datacompy compareCLI (closes #530), enabling dataset comparisons from the shell and CI/CD pipelines without writing Python.datacompy/cli/subpackage:parser.py,compare.py,backends.py,loaders.py,sessions.py,output.py,errors.pydb.schema.table) or stage local files to a temp table0match,1mismatch/threshold violated,2error--max-unequal-rows Nfor threshold-based CI assertions--jsonfor machine-readable output;--quietfor exit-code-only pipelines--debugre-raises unexpected exceptions for bug reportingtype=callables (value rules) and_validate_arg_combinations(cross-argument rules)contextlib.ExitStack— Spark and Snowflake sessions close on both success and failuredatacompy --versiondoes not pay the pyspark/snowflake import cost