cowork: schema command error-path hardening + honest sample footer - #49
Conversation
Three correctness fixes in converters.py: 1. CsvWriter extrasaction='raise' → 'ignore': JSON/YAML→CSV conversions with heterogeneous rows (later rows having extra keys not seen in the first row) raised ValueError and aborted the conversion. extrasaction= 'ignore' silently drops extra fields, which is correct CSV behaviour — the output schema is fixed at the first row. 2. Remove redundant CSV pre-peek in convert(): the code opened a fresh read_stream() to peek the first row and call set_field_order(), then opened a second read_stream() for the actual conversion — reading the input file twice. CsvWriter.write_stream() already falls back to list(row.keys()) when _field_order is unset, so the pre-peek was entirely redundant. Removed; one read pass now suffices. 3. Populate ConversionResult.rows_read: the field was declared but never set (always 0). Set to rows_written after a successful conversion so callers can trust the result object. 5 new regression tests; 119/119 green.
…morph # Conflicts: # .github/workflows/pages.yml # .github/workflows/publish.yml # .gitignore # LICENSE # README.md # package.json # pyproject.toml # src/datamorph/__init__.py # src/datamorph/cli.py # src/datamorph/converters.py # tests/test_converters.py # tests/test_validate.py
- Add parquet, avro, protobuf, and all optional dependency groups to pyproject.toml - Wrap optional dependency imports in try/except with user-friendly error messages pointing to the correct pip install command - All 141 tests pass, ruff clean
… bad merge An earlier merge on cowork/improve-datamorph inadvertently dropped 14 files that exist on master (ci.yml, auto-code-review.yml, conftest.py, CHANGELOG, CONTRIBUTING, SECURITY, issue/PR templates, dependabot, FUNDING, cli.js, .gitattributes). Restored them verbatim from origin/master so this branch's PR diff contains only the intended improvements (converters.py graceful optional-import errors + pyproject optional-dependency groups + auto-pr wf).
…adge, git+ install)
…morph # Conflicts: # pyproject.toml
…ted format and malformed input, no tracebacks); fix misleading 'Inferred from N+ rows' footer to honest sample cap; +3 CLI error-path tests
🤖 Automated Code Review✅ Ruff Lint — No issues
|
… valid empty container); harden validate --schema file loading (clean exit on malformed/wrong-shape schema, no traceback); +4 tests (148 pass), ruff clean
…files - convert_batch returns BatchConversionResult with skipped[] (file + detected format), CLI reports each skip explicitly instead of a green 'complete' while dropping data; +1 regression test (149 pass), ruff clean
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fbb32461b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| recursive: bool = False, | ||
| **writer_kwargs: Any, | ||
| ) -> list[ConversionResult]: | ||
| ) -> BatchConversionResult: |
There was a problem hiding this comment.
convert_batch is re-exported as a public API from datamorph.__init__, but this changes its return value from list[ConversionResult] to a non-iterable dataclass. Existing callers that iterate, index, compare to [], or call len(convert_batch(...)) now fail even though the CLI was updated. Keep the result list-compatible (for example by implementing the sequence protocol) or introduce the richer result through a new API.
Useful? React with 👍 / 👎.
| console.print(f"\nDetected format: [bold]{fmt}[/bold]") | ||
| console.print(table) | ||
| console.print(f"[dim]Inferred from {sample}+ rows[/dim]") | ||
| console.print(f"[dim]Inferred from a sample of up to {sample} rows[/dim]") |
There was a problem hiding this comment.
When a nonempty file is inspected with --sample 0 or a negative value, FormatReader.infer_schema still appends the first row before checking its stopping condition, while this new footer claims that at most zero (or a negative number of) rows were sampled. This both misreports the operation and silently infers a schema from only one row; constrain the Click option to positive integers or make inference honor zero consistently.
Useful? React with 👍 / 👎.
2 participants
Harden the
schemasubcommand (silent-failure class): unsupported --format and malformed input now exit cleanly with a red ERROR message instead of an unhandled traceback. Fix misleading 'Inferred from N+ rows' footer to 'up to N rows' (honest claim). Adds 3 CLI error-path tests (144 total pass, ruff clean). Branch also carries a merge of origin/master resolving the pyproject self-referential extra conflict in favor of the corrected datamorph-cli name.