Developer1010x commented on PR #1732: URL: https://github.com/apache/datafusion-python/pull/1732#issuecomment-5664440308
Fair question, and I would rather answer it straight than dance around it: this was written with an AI agent. The commit carries a `Co-Authored-By: Claude` trailer for that reason, so you are not wrong about any of the three. The distinction I would offer is not "mine is not AI" but that it was not one pass over the issue text. Against #1736, which is the closest of the three: - **Coverage.** My step runs *after* the tpchgen-cli step, so `python-udf-comparisons.py` is exercised rather than skipped. It reads `examples/tpch/data/lineitem.parquet`, which that step creates. #1736 places the job before it and skips the example for that reason. 14 examples run instead of 13. The issue does not mention this dependency, it only shows up once you actually run them. - **`substrait.py`** reads `./testing/data/csv/aggregate_test_100.csv`, so it only works when the working directory is the repository root. I resolve it relative to `__file__`. Both approaches pass in CI; only one works when a user runs the file directly. - **Example 3 of `csv-read-options.py`** configures `delimiter="|"` and `quote="'"`. #1730 and #1736 point it at the comma-separated fixture, so it runs green and prints a single-column table, demonstrating the opposite of what it sets. I write a pipe-delimited file for that example and a gzipped one with a comment line for the advanced-options example. - **`examples/README.md`** links `sql-on-polars.py`, `sql-on-pandas.py` and `sql-on-cudf.py`. None of the three are in the repository. Removed, and replaced with a table of what each example needs. One point where #1736 is better than mine: it gates on `matrix.python-version == '3.12'`, which runs the job once. I followed the neighbouring `matrix.wheel-tag == 'abi3'` convention, and that matches five matrix rows, so my job runs five times where once is enough. Their gate is the correct one and I am happy to switch to it. On "a single output of dumping the issue into an agent": the thing that argues against that, for whatever it is worth, is what running the examples turned up. `CsvReadOptions.with_null_regex` is accepted and silently ignored (#1735). That traced to `datafusion-datasource-csv`, where `CsvSource::builder` never passes the regex to the `ReaderBuilder` that parses the rows, so it applies during schema inference only: apache/datafusion#25213, fix in apache/datafusion#25254. Chasing that turned up a second, unrelated defect on the same path, where an invalid pattern panics the query task instead of returning an error: apache/datafusion#25260, fix in apache/datafusion#25261. None of that is in issue #1728. Closing all three is a perfectly reasonable call and I will not take it badly. If it is more useful, I am glad to cut this down to just the CI job, with their gate, so there is a smaller change for someone to review properly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
