Declare subpackages instead of shipping the source tree as package data - #216
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Reviewed with a fresh pass, including building wheels from clean and deliberately-dirty trees rather than taking the numbers on trust. The core claim is confirmed, and the mechanism is worse than the description says in one respect and better in another. Neutering only the data glob on the base tree collapses the wheel to 6 files, 9,408 bytes — proving all 25 subpackage modules were shipping as package data rather than declared code. And the fix works: with But the bloat is latent, not shipped. The published 3.1.1 wheel on PyPI is 127,432 bytes — clean. I could not reproduce the 501 KB figure: I measured 361 KB with one interpreter's To doBefore merging
Optional
Verified, no action needed
|
The previous approach reformatted five documentation files and bounded ruff to <0.17.0. Per review, the bound was a no-op against the failure it targeted: 0.16.0 through 0.16.8 all format markdown and all satisfy it, so only the reformatting was doing any work - and the next formatter change inside 0.16.x would reopen the failure. Excludes docs/**/*.md via [tool.ruff] instead, which fixes the cause rather than the symptom, and drops the five reformatted files. That also removes the byte-identical docs hunks that collided with #216, #217 and #219. The dev extra is bounded to match CI, so a contributor running make format no longer reformats files CI then rejects. The cause was ruff 0.16.0, not 0.16.7: 0.13, 0.14 and 0.15 report '65 files already formatted' and do not scan markdown at all.
|
Review of 019aaaa: No implementation defect found in this PR. The remaining merge blocker is the inherited Lint failure. Merge blocker — incorporate #218 and rerun CI. The Lint job fails because Ruff would reformat five unchanged Markdown files: Validation at the reviewed head: built an sdist and then a wheel from that sdist. Both retained all 27 expected Python modules and Integration: read-only merge checks found no textual conflicts in the cumulative #218 + #214 + #215 + #216 + #217 tree. This is not a combined-suite result. #219 conflicts with #214, #215, #217, and #218, so its later rebase must retain the seed, Matching, API, and formatter fixes. The PR description cites an older final commit and historical green checks; update it to the resulting implementation and current validation. |
packages = ["microimpute"] with package-data "**/*" meant the subpackages reached the wheel only as package data, swept in by a glob that also collected whatever else was in the working tree. A wheel built from a tree with compiled bytecode present carried 27 __pycache__ entries and around 500 KB of build-host bytecode, so wheel contents were a function of the builder's working directory rather than the source. setuptools.packages.find declares them properly. Verified: with 54 .pyc files present in the tree, the built wheel now contains none, and every subpackage imports from the installed wheel. Also adds py.typed, so the annotations become visible to downstream consumers; the two missing authors, which left the paper's corresponding author out of the PyPI metadata; and classifiers and project URLs, with no repository link previously on the PyPI page. The absent LICENSE file is #197 and is not addressed here. Fixes #211
Per review. Adds Python 3.12/3.13/3.14 classifiers to match requires-python, an email for Vahid so all four authors render in Author-email rather than one splitting into the legacy Author field, and an explicit exclude alongside the include as belt and braces. Rewords the changelog fragment: the bloat is latent rather than shipped. CI builds from a fresh checkout with no imports, so published wheels have been clean - the problem appears when anyone builds from a tree that has been tested in. The previous wording implied released wheels were affected. Also drops the docs reformatting, which belongs to #218.
019aaaa to
6a0e9b7
Compare
Critical fixedUpdated the branch onto current Should-address fixedRefreshed the description to state the dirty-build contamination risk and current validation evidence. License work remains separate in #197. Verification
Pushed head: GitHub CI: All seven current-head checks passed, including lint, changelog, Python 3.12 smoke tests, Python 3.14 tests with R, and Vercel preview. GitHub reports this pull request as mergeable. (run). |
Fixes #211.
Setuptools now discovers
microimputeand its subpackages explicitly and excludes tests, documentation, examples, and experiments from package discovery. Replacing the recursive package-data glob with an explicitpy.typedentry prevents bytecode and stray working files from entering distributions. Package metadata lists the four authors, Python 3.12–3.14 classifiers, and repository, documentation, and issue links.The packaging change addresses contamination from a dirty build tree. It does not establish that published wheels were contaminated. License work remains separate in #197.
Package-layout validation built a source distribution and then a wheel from it. Both retained all 27 expected Python modules and
py.typed, and excluded injected bytecode and stray CSV/JSON files. Six imports resolved to the extracted wheel outside the checkout; an independent OLS numerical check and two artifact smoke tests passed. These checks preceded the base rebase; verification confirmed unchanged package-discovery/data settings and source contents, with only upstream version and lint metadata updates.The branch includes #218's shared lint repair; Ruff 0.16.7 formatting and
git diff --checkpass.Checks for current head
6a0e9b7.