Pin the dependency set, replace the dead profiler, stop a test destroying data - #19
Merged
Merged
Conversation
…ying data Three things, all found by actually installing and running rather than reading. requirements.txt was entirely unpinned on a repository marked Stable. The maintenance policy asks that a clone still run years from now, and an unpinned set nobody is watching is a CI failure waiting for a quiet week. Every version here was resolved together and verified on Python 3.11 with the suite green. pandas-profiling is worse than deprecated. The maintainers renamed it to ydata-profiling at version 4 and no longer publish the old name; on Python 3.11 it fails to install outright, because its htmlmin dependency cannot build a wheel. CI survived only because it pinned Python 3.10. eda/profile_data_quick.py imported it at module scope, so that module was one Python bump from being unimportable. Swapped for ydata-profiling 4.18.4, verified installing and importing in a clean virtualenv, and the workflow moves to 3.11 so what CI runs is what was tested. tests/test_read_large_csv_chunked.py opened sample_data/gender_sample.csv in write mode and overwrote the committed sample data with "a,b\n1,2\n3,4\n5,6". Every run of the suite destroyed it. It survived only because nobody committed the damage; I found it staged in my own working tree. It now writes to pytest's tmp_path, and a second test asserts the sample file's hash is unchanged after a run, so this cannot come back quietly. test_export_formatted_excel.py wrote into the repository root and removed the file afterwards, which leaves it behind whenever the assertion fails. Also moved to tmp_path. Also here: the regression test pinning survey_estimation to R's survey 4.2.1. The cross-validation was run last session and agreed to twelve significant figures, but the commit that landed did not include the test, so main carried the claim without the guard. It does now, against a dataset built with no random number generator so the reference values stay valid across library versions. README claimed scikit-learn as a key dependency; it is neither declared nor imported anywhere. Corrected, along with the rest of that list. 26 tests green against the exact pinned file. CLAUDE.md records the pinning rationale, the pydantic @validator deprecation that will bite whoever raises pydantic to 3, the invariants that must be re-run before touching the estimator, and the owner's design reference for UI work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DMYhECx2Dzbqad6KJxWZ7t
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Three things, all found by installing and running rather than by reading.
requirements.txtwas entirely unpinned on a repository marked Stable. The maintenance policy asks that a clone still run years from now, and an unpinned set on a repository nobody is watching is a CI failure waiting for a quiet week. Every version is now pinned, resolved together, and verified on Python 3.11 with the suite green.pandas-profilingis worse than deprecated. The maintainers renamed it toydata-profilingat version 4 and no longer publish the old name. On Python 3.11 it does not install at all: itshtmlmindependency cannot build a wheel. CI survived only because it pinned Python 3.10.eda/profile_data_quick.pyimported it at module scope, so that module was one Python bump away from being unimportable. Swapped forydata-profiling==4.18.4, verified installing and importing in a clean virtualenv, and the workflow moves to 3.11 so what CI runs is what was tested.A test was destroying committed data.
tests/test_read_large_csv_chunked.pyopenedsample_data/gender_sample.csvin write mode and overwrote it witha,b\n1,2\n3,4\n5,6. Every run of the suite destroyed the sample file; it survived only because nobody ever committed the damage. I found it staged in my own working tree. It now uses pytest'stmp_path, and a second test asserts the sample file's hash is unchanged after a run so this cannot come back quietly.test_export_formatted_excel.pywrote into the repository root and removed the file afterwards, which leaves it behind whenever the assertion fails; also moved totmp_path.A correction to #18
#18 said the R reference values were hardcoded into the test suite. The cross-validation against R's
survey4.2.1 was genuinely run and agreed to twelve significant figures, but the commit that landed did not include the test, somainhas been carrying the claim without the guard. This PR adds it, against a dataset built with no random number generator so the reference values stay valid across library versions.README.mdalso claimedscikit-learnas a key dependency; it is neither declared nor imported anywhere. Corrected along with the rest of that list.Type of change
Checklist
requirements.txt, in a clean virtualenv on Python 3.11ydata-profilingverified installing and importing from a fresh venv;eda/profile_data_quick.pyconfirmed importable againpandas-profilingfailure reproduced, not assumed:ERROR: Failed building wheel for htmlminsample_data/confirmed unmodified after a full suite runCLAUDE.mdrecords the pinning rationale and the pydantic@validatordeprecation that will bite whoever raises pydantic to 3🤖 Generated with Claude Code
https://claude.ai/code/session_01DMYhECx2Dzbqad6KJxWZ7t
Generated by Claude Code