Conversation
Prepares the repository for submission to the Journal of Open Source Software (https://joss.readthedocs.io/en/latest/submitting.html). Paper: - paper/paper.md with all currently-required JOSS sections: Summary, Statement of need, State of the field, Software design, Research impact, and AI usage disclosure (~1,240 words, within the 750-1750 limit). Salvages the partial draft on the `joss` branch. - paper/paper.bib, curated to the 20 entries actually cited, with DOIs and full venue names. Verified to resolve under pandoc --citeproc. - .github/workflows/draft-pdf.yml to compile the paper in CI. - paper/SUBMISSION_CHECKLIST.md tracking the remaining human-only steps (missing ORCIDs, author list, AI disclosure sign-off, Zenodo DOI). Community guidelines (a required review-checklist item): - CONTRIBUTING.md, CODE_OF_CONDUCT.md, CITATION.cff, issue and PR templates. Documentation (required: statement of need, install, example usage, API): - docs/{index,installation,quickstart,user_guide,api_reference,datasets}.md - README rewritten with badges, a statement of need, a runnable quickstart, a method table, and citation info. Tests: 29 -> 116, and four 0-byte stub files removed or filled. - Fill tests/test_dnn_kernel.py and tests/test_dnn_wasserstein.py. - Add tests/test_{datasets,fit_methods,public_api}.py. - Delete tests/test_syn_nn.py (no such module) and tests/test_nadaraya_watson.py (target cannot be instantiated). - test_precommit.py now skips when pre-commit is absent or off Linux. CI: test on Python 3.10-3.12 plus macOS and Windows; split lint into its own job; pin actions to v4/v5. Fixes found while documenting the public API: - get_available_datasets() returned [] unless a loader had already been imported; it now discovers dataset subpackages. - nsquared/__init__.py exports an explicit, documented __all__. ts_nn and aw_nn resolved to modules rather than the constructor functions. - NadarayaWatsonEstimator is excluded from the public API: it never implements the abstract _calculate_distances, so it cannot be instantiated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JOSS's generative AI policy asks for three things: the AI systems and versions used and exactly where they were applied, the scope of that assistance, and an assertion of human review and validation. The prior disclosure named no tool. Rewrite it to that structure, centred on test generation, which is where the assistance actually went. The manuscript-drafting sentence is bracketed: delete it if the paper is rewritten from scratch, keep it if AI-drafted prose survives revision. Also fix three pyright errors in the new test files. These were missed because `pre-commit run --all-files` only checks git-tracked files, and the test files were still untracked when the hooks were last run, so the CI lint job would have failed: - test_datasets.py passed a dict[str, int] via ** into NNData.create, which pyright could bind to the bool/str parameters. - test_fit_methods.py unpacked FitMethod.fit's union return type without narrowing it first. Narrowing the reproducibility test also exposed that SyntheticDataLoader seeds the global NumPy RNG in its constructor rather than holding its own Generator, so same-seed loaders only reproduce under construct-then- generate ordering. The test documents the constraint; a fix is tracked in the submission checklist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An earlier append landed on the file's last line, which had no trailing newline, turning the commented-out `# *.npy` into `# *.npy/.venv/`. Restore it. No functional change: .venv is already ignored at line 126. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two packaging problems, both user-visible in the published 1.1.0 wheel. BREAKING: `baselines` is now `nsquared.baselines`. The wheel shipped a top-level `baselines` package, so `pip install nsquared` squatted a very generic name in the user's environment -- one that collides with OpenAI's RL library of the same name. Moved under `nsquared/` and added `include = ["nsquared*"]` to the setuptools package finder so a stray top-level package cannot be shipped again. The seven scripts in examples/ are updated. BREAKING: most dependencies are now optional extras. The core install required 13 packages -- including `wrds` (needs credentials), HuggingFace `datasets`, `fancyimpute`, and `SyntheticControlMethods` -- for someone who only wanted `row_row`. Four of them (`seaborn`, `tabulate`, `SyntheticControlMethods`, `wrds`) were never imported by the library at all, only by scripts in examples/. Required is now numpy, hyperopt (threshold search), tqdm (progress). The rest are behind `data`, `baselines`, `plots`, and `examples` extras; `[all]` restores the old behaviour and `[dev]` implies it. Verified in a clean venv: a bare install pulls 7 packages (was 60+) and still does scalar and distributional imputation, cross-validation, and synthetic data. Missing extras now fail with an actionable message rather than a misleading one. `NNData.create` distinguished nothing before: a typo and a missing dependency both produced "Dataset X not found". It now separates the two using find_spec, names the extra to install, and no longer records bogus names in the registry that `NNData.help()` then listed as unavailable. Two bugs found while doing this: - Lazy-loading `softimpute` returned the function on first access and the implementation *module* on every access afterwards, because the two shared a name and the import system rebinds the parent attribute. Renamed the module to `_softimpute` so the binding is unambiguous, and added a TYPE_CHECKING import so pyright still sees a function. - SoftImpute was already broken before this change: fancyimpute 0.7.0 calls `check_array(force_all_finite=...)`, removed in scikit-learn 1.8. Pinned `scikit-learn<1.8` in the baselines extra, with a test that runs it end to end so the pin cannot silently rot. Tests 137 -> 144. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fancyimpute was a bad trade: 26 transitive packages -- including four convex solvers the algorithm never uses, plus nose and pytest as runtime deps -- for a ~40 line algorithm. Its 0.7.0 release is also broken against scikit-learn >= 1.8, which removed the `force_all_finite` argument it passes to check_array, so the SoftImpute baseline raised TypeError on any fresh install. The previous commit worked around that with a `scikit-learn<1.8` pin; this removes the cause instead. Implemented from the published algorithms rather than vendored, so no Apache-2.0 code enters an MIT repo: iterative soft-thresholded SVD from Mazumder, Hastie & Tibshirani (2010), and the row/column bi-scaling from Section 8 of Hastie et al. (2015). Both papers were already cited. Numerically equivalent to what it replaces. Across 12 randomized problems varying size, rank, noise and missingness, the worst relative difference on imputed entries against fancyimpute is 2.6e-4, and RMSE against ground truth agrees to ~1e-4 -- so published benchmark numbers stand. One deliberate behaviour change: a row or column with no observed entries now raises ValueError. fancyimpute rejected the same input; an earlier draft of this implementation instead returned values near the global mean, which look like estimates but carry no information. Consequences: - The `baselines` extra is gone; both baselines need only NumPy. The lazy-loading machinery added for fancyimpute is gone with it, along with the module/function shadowing it required working around. - A bare `pip install nsquared` is 7 packages and now includes working USVT and SoftImpute. Tests 144 -> 159, including recovery of low-rank structure, parameter effects, determinism, and the rejected-input cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nsquared/simulations/ was dead code -- nothing outside the package imported
it -- while the synthetic loader carried its own private copy of the same
ideas. Since __init__.py exports `simulations`, that made dead code part of
the documented public API.
Wiring the loader to call the module as it stood would have been a
downgrade, because the module was both poorer and broken:
- gendata_s_adopt had never run. It allocated Data/true_Mean/true_Cov as
(N, T) but assigned (n, d), (d,) and (d, d) values into scalar slots, so
it raised ValueError on its first iteration. It also indexed u_1[i + 1]
past the end on the last row, and passed size=1 to np.random.binomial and
assigned the resulting length-1 array into a scalar slot.
- Both MCAR generators returned a "true" matrix that was not noiseless:
`Theta = Y` binds the same array, and the following `Y += noise` mutated
it in place, so anything scoring against Theta scored against noise.
So the direction is inverted: the loader's better generative model (the
additive Holder signal, configurable rho/snr, nonlinear transforms, correct
data_true/data_noisy split) moves into nsquared/simulations/latent_factors.py,
and the loader now imports and calls it. Same end state -- one
implementation, no dead code -- without a regression.
Verified equivalent: data, mask, true, noisy, and both latent factor
matrices hash identically before and after across six configurations
spanning seeds, snr, additive/multiplicative models, latent dimension and
noise level.
Fixed along the way:
- simulated_data_nonlin_transform was a complete no-op. The loader called
_transform_simulated_data(Y) and discarded the return value, so "tanh" and
"cubic" produced data byte-identical to no transform. Nothing in examples/,
bench/ or tests/ passed it, so no published result depended on it.
- expit now evaluates branchwise so neither tail overflows.
- Removed a stray `print("Additive model")` and the loader's now-unused
private helpers.
gendata_s_adopt now runs and returns its documented shapes, but only the
crashes were fixed -- whoever owns the MNAR work should review the
statistical design. MNAR is still not wired into the loader, which continues
to raise NotImplementedError.
Tests 159 -> 180.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#71 asked for two specific clarifications about the mask. Both were verified against the loaders rather than restated from the issue: - The mask encodes *treatment*, not "is this cell populated". The HeartSteps loader derives it from the send.sedentary treatment indicator, so mask == 0 means "not treated here, and their outcome under treatment is the estimand", not "nothing is known here". - mask == 0 does not imply nan in the data matrix, and the loaders genuinely disagree: prompteval keeps the held-out truth in place (its `data[mask == 0] = np.nan` is commented out on purpose so entries can be scored), while synthetic_data does write nan. Inferring missingness with np.isnan(data) therefore reads the answer being predicted on some datasets, silently turning a benchmark into a leak. Documented in docs/user_guide.md with a worked wrong/right snippet, and linked from the quickstart and datasets pages where the trap is reachable. #111 asked for a KNNImputer baseline. Implemented on NumPy rather than adding scikit-learn back as a dependency, which would have undone the dependency slimming: knn_impute matches sklearn.impute.KNNImputer's semantics, including the nan-aware Euclidean metric, the restriction to donor rows that observe the target column, and both uniform and inverse-distance weighting. Verified equal to scikit-learn to machine precision (worst 8.9e-16 across randomized shapes, both weightings, k of 1, 3 and 5); the comparison is kept as a test that skips when scikit-learn is absent. Also adds knn_impute_columnwise, since matrix completion has no privileged orientation and the row-only form is precisely what this package's estimators generalize. Tests 180 -> 199. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every push to this branch failed CI and I misread the Actions page as green; the API shows five consecutive failures. Cause was mine. Rewriting ci.yml earlier replaced `python -m venv .venv` + `pip install .` with a bare `pip install -e ".[dev]"` into the runner's Python. But [tool.pyright] in pyproject.toml pins venv = ".venv" / venvPath = ".", and the pre-commit pyright hook runs inside pre-commit's own isolated environment, so it cannot see the ambient interpreter. With no .venv at that path pyright resolved nothing and reported 193 reportMissingImports errors. Reproduced locally by running the hooks against a checkout with no .venv. That failed the lint job, and cascaded into the three Ubuntu test jobs through tests/test_precommit.py, which runs pre-commit and is Linux-only -- which is why macOS and Windows were the only green jobs. Removing the venv pin does not work: without it pyright picks up pre-commit's own interpreter, which has no dependencies, and still reports 193 errors. The pin is load-bearing, so instead: - The lint job now builds .venv and runs pre-commit from it, matching what the pyright config expects. Verified against a CI-shaped checkout. - The test jobs keep the fast ambient install, and test_precommit.py skips when there is no .venv to point pyright at, with that stated as the skip reason. Linting is the lint job's responsibility; running it once per matrix entry was redundant anyway. - [tool.pyright] include now lists tests as well as src, matching the files pre-commit actually passes it, and carries a comment explaining why the venv pin cannot be dropped. Also folds in the kernel work in flight: - Every kernel in utils/kernels.py did `dists /= eta`, mutating the caller's array in place, so a caller's distance matrix came back silently divided by the bandwidth. Now computed out of place. - epanechnikov and wendland already existed but were never reachable; both are wired into NadarayaWatsonEstimator.valid_kernels, closing the "implement additional kernel functions" TODO. TODOs in src/ 19 -> 8; the rest map to open issues or need an owner's call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
My CI-reproduction step copied the worktree with tar without excluding .git. In a git worktree .git is a file pointing back at the real gitdir, so the `git init` + `git add -A -f` I ran inside the scratch copy operated on this repository instead. That produced the stray "ci" commit, which forced 70 ignored files into tracking: .pytest_cache/, .ruff_cache/, .joblib_cache/, and a complete build/lib/ copy of the package. build/, .pytest_cache/ and .joblib_cache/ were already in .gitignore and only landed because of the -f. .ruff_cache/ was genuinely missing and is now added. Files are untracked here, not deleted from disk. The stray commit stays in history; its real content (the kernels and mcar changes) is intact at HEAD. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both README and CONTRIBUTING told contributors to run pre-commit without mentioning that [tool.pyright] pins venv = '.venv'. Installing the package anywhere else -- conda, system Python -- leaves pyright with no environment to resolve against, so it reports every third-party import as unresolved. That is what broke CI, and it would look like hundreds of real errors to anyone hitting it. Both files now say so explicitly. Also corrected two stale claims: CONTRIBUTING said a lint failure is always a test failure (test_precommit.py now skips without ./.venv or off Linux), and both files described CI as running everything across 3.10-3.12 (tests run there plus macOS and Windows; linting runs once in its own job). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The lint job kept failing while every test job passed. Cause was a single pyright error in SyntheticDataLoader.get_full_state_as_dict that is invisible on Python 3.12 and fires on 3.10. return_dict is initialized with array values only, so its value type is inferred as NDArray, and the metadata dict assigned into it afterwards is not assignable. Whether pyright reports that depends on the numpy stubs: Python 3.10 resolves numpy 2.2 (2.3+ requires 3.11), which infers the narrow type, while 3.12 gets numpy 2.5, which does not. Annotating the dict explicitly makes it correct regardless of interpreter. Found by reproducing the lint job against a pyenv 3.10.19 clone, after a 3.12 reproduction passed and hid it. Verified on both 3.10 and 3.12. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Estimators: aw_nn and NadarayaWatsonEstimator returned nan whenever the data held nan at masked positions, which is the synthetic loader's own convention. AutoEstimator now forwards allow_self_neighbor to its doubly robust component. Cross-validation: rng on the dual-threshold fitters, TS fitter no longer drops ret_trials, evaluate_imputation returns nan without a RuntimeWarning. Loaders: save_dir was swallowed by **kwargs; raw files now land there. Quiet joblib caches, Prop 99 declared params match the constructor, PromptEval scalar mode raises ValueError with guidance instead of an AssertionError. Packaging: nsquared.__version__, explicit datasets exports, empty helper_fns removed, release workflow installs the dev extras so its tests can run. Docs and examples corrected to match the code; regression tests for each fix. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GREFJedNXpqYHztKSnCcaT
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GREFJedNXpqYHztKSnCcaT
…ations Rewrite the AI usage disclosure to cover everything on this branch, naming both models and the July-September window. Cite fancyimpute and syntheticNN, count the baselines correctly, add the missing DOI, and set the date. Bump the version to 1.2.0 in pyproject.toml and CITATION.cff. The release workflow now skips an already-uploaded version instead of failing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GREFJedNXpqYHztKSnCcaT
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GREFJedNXpqYHztKSnCcaT
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
1 participant
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.
Prepares the repository for submission to the Journal of Open Source Software.
Paper
paper/paper.md+paper/paper.bib(1506 words; all citations resolve; Draft-PDF workflow added as.github/workflows/draft-pdf.yml).paper/SUBMISSION_CHECKLIST.mdlists the remaining human-only items (ORCIDs for Chin, Maskara, Agarwal; author list; Zenodo DOI).Breaking changes (hence 1.1.0 → 1.2.0)
baselinesmoved into the package namespace:from baselines import usvt→from nsquared.baselines import usvt. 1.1.0 shipped a top-levelbaselinespackage on PyPI.pip install nsquaredpulls onlynumpy,hyperopt,tqdm; the loaders neednsquared[data];nsquared[all]restores the old behaviour.fancyimputedropped. SoftImpute is implemented on NumPy (nsquared/baselines/_softimpute.py), matching the original to 2.6e-4 relative error; k-NN imputation added, matchingsklearn.impute.KNNImputerto machine precision.Fixes
NadarayaWatsonEstimatorwas not instantiable; now complete, withepanechnikov/wendlandkernels.aw_nnand Nadaraya–Watson returnednanon data withnanat masked positions (the synthetic loader's own convention).get_available_datasets()returned[]; loaders are now discovered automatically and a missing extra raises aValueErrornaming it.save_dirwas silently ignored by every loader; raw files now land there.rngon all cross-validators (TS/DR/Auto were non-reproducible); TS fitter droppedret_trials.gendata_s_adopthad never run (three crashes);simulated_data_nonlin_transformwas a no-op; kernels mutated caller arrays in place;AutoEstimatorignoredallow_self_neighbor.help()defaults now match their constructors; PromptEval scalar mode raises a clearValueErrorinstead of anAssertionError.Docs, community, packaging
docs/(index, installation, quickstart, user guide, API reference, datasets), rewrittenREADME.md,CONTRIBUTING.md,CODE_OF_CONDUCT.md,CITATION.cff, issue/PR templates.nsquared.__version__added.Closes #71, closes #111.
After merging:
git tag -a v1.2.0 -m "N^2 1.2.0" && git push origin v1.2.0, then draft the GitHub release from that tag (not prerelease) sopublish.ymluploads to PyPI.🤖 Generated with Claude Code
https://claude.ai/code/session_01GREFJedNXpqYHztKSnCcaT