Conversation
pysum is broken on modern pandas, and repairing it is the less interesting question. Measured against pandas 3.0.5: summarizeDF raises AttributeError on any plain DataFrame, and -- worse -- pandas 3 gives string columns the dtype `str` rather than `object`, so the code's `== 'object'` branch never fires and every categorical column silently loses its value listing and frequency table. It runs, emits a table, and omits the analysis. pytablewriter's header_list was removed in 1.x, so headers degrade to A B C D E F. xlsx raises ModuleNotFoundError for an undeclared xlsxwriter, and html shells out to pandoc through os.system, which fails silently when pandoc is absent and interpolates a user-supplied filename into a shell command. There are no tests. The interesting question is what it should be instead, and the answer came from the data skills rather than from the package. audit-analysis carries audit_data.py: 433 lines, pandas-only, untested and unpackaged, encoding real opinions that nothing on PyPI ships -- differential missingness manufactures trends where constant missingness only attenuates; skew above 2 or max/median above 20 changes which method is defensible. And build-data makes sentinel codes its first checkpoint while nothing anywhere implements sentinel detection. So pysum becomes a diagnostic profiler rather than another descriptive summariser. skimpy already owns the console summary and supports polars; fg-data-profiling owns the heavy report and is now partly commercial. Nobody ships the thing that says this missingness differs by arm, this column spikes at -999, these zeros are probably missing. Everything it reports is a candidate rather than a verdict, which is audit_data.py's stance and the right one: the tool states the fact, the analyst decides. Hard checks -- a non-unique key, a share outside [0,1] -- exit non-zero so it can gate a pipeline. The rest need judgement. The API stops writing files as a side effect and returns an object instead, rendering and writing being separate decisions. No monkey-patched df.name. pandas and polars from the start through narwhals, and no more comparing dtypes against strings, which is precisely what pandas 3 broke. Success is checkable: audit_data.py gets deleted and the skill installs pysum instead. Until pysum reproduces its findings on data with known problems, the rewrite has not earned its thesis. Every check needs a fixture that trips it and one that does not. No code yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![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: fe90801c03
ℹ️ 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".
| ```python | ||
| import pysum | ||
|
|
||
| report = pysum.profile(df) # pure; returns an object, writes nothing |
There was a problem hiding this comment.
With only a dataframe argument, the profiler cannot know which columns are declared keys or groups, which numeric columns are shares/counts, how rates map to denominators, or what regression defines the headline outcome. Consequently checks 1, 2, 6, 8, and 9 must either be skipped or infer semantics from column names, and inferred range violations could incorrectly trigger the promised non-zero exit. Define schema/model configuration in the public API before treating these checks as implementable.
Useful? React with 👍 / 👎.
| report.findings # list of Finding(check, column, severity, message) | ||
| report.variables # the per-variable table | ||
| print(report.summary()) # the console slice | ||
| report.to_markdown(path) / .to_html(path) |
There was a problem hiding this comment.
Passing path to to_markdown or to_html makes the proposed rendering method perform the disk write itself, directly contradicting the rule below that rendering and writing are separate calls. This leaves implementers unable to determine whether these methods return rendered content, write a file, or do both; expose a path-free renderer and a distinct write/export operation.
Useful? React with 👍 / 👎.
The ten-check draft made pysum a general profiler, which means competing with skimpy from behind. The line that decides scope: pysum discovers, pandera verifies. A check that needs you to declare the key or the range first is confirming something you already suspected, and pandera does that better. Four checks survive it -- sentinel candidates, differential missingness, zeros-vs-missing, dtype traps. None require a declaration, and none are served by a schema validator, which structurally cannot see a relationship between missingness and treatment arm. Also corrects a false claim. The draft said nothing implements sentinel detection; profile_columns.R does, thoroughly. The true claim is that nothing packaged does, and that naniar's miss_scan_count needs you to supply the values, so it counts rather than discovers.
1 participant
docs/PRD.md. No code — the decision first, per "come up with what we want and then execute against it".pysum is broken, measurably
Against pandas 3.0.5 / pytablewriter 1.2.1:
summarizeDF(df)→AttributeError: 'DataFrame' object has no attribute 'name'. Documented in the README, so a deliberate API choice — but it asks users to monkey-patch a pandas objectstr, notobject. The code branches on== 'object', so every categorical column silently loses its value listing and frequency table. It runs, emits a table, omits the analysiswriter.header_listremoved in pytablewriter 1.x → headers degrade toA B C D E FModuleNotFoundError: xlsxwriter. html shells out topandocviaos.system— fails silently when absent, and interpolates a user-supplied filename into a shell commandfind . -name 'test*.py'returns nothingThe finding that decided the product
It came from your data skills, not the package.
audit-analysis/scripts/audit_data.pyis 433 lines, pandas-only, untested, unpackaged, and it encodes opinions nothing on PyPI ships:And
build-datamakes sentinel codes its first checkpoint (-999,999,88) — while nothing implements sentinel detection.So: not another descriptive summariser. skimpy owns the console summary and supports polars; fg-data-profiling owns the heavy report and is now partly commercial. Nobody ships the diagnostic — this missingness differs by arm, this column spikes at -999, these zeros are probably missing.
What the PRD settles
audit_data.py's stance and the right one. The tool states the fact; the analyst decides.[0,1]) exits non-zero so it can gate a pipeline.profile()returns an object and writes nothing. Rendering and writing are separate decisions. No monkey-patcheddf.name. HTML in-process, no pandoc.build-data's rule.Success criteria
audit_data.pygets deleted and the skill installs pysum instead. Until pysum reproduces its findings on data with known problems, the rewrite hasn't earned its thesis. Every check needs a fixture that trips it and one that doesn't — a diagnostic that never fires and one that always fires are equally useless.Read it and push back before Phase 2 writes any code.
🤖 Generated with Claude Code