Repository navigation
Conversation
Pickle reading hardened (F038, F040, F140, security). The pycisTopic reader checks what it hands numpy, scipy and pandas, and no longer allows dates and times. pd.NA and empty bytes, which pandas 1.5 and old protocols write, now read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tx (F007, F025, F100) An imputed pycisTopic matrix is written BLOCK_ENTRIES (1M) entries at a time, a block of cells or a run of one cell's features, and that is also the dense output's chunk: neither grows with the number of features, so HDF5 no longer refuses the chunk past 262,144 float32 features (F007) and CSR output no longer holds 4096 cells of coordinates (F025). A sparse mtx asked for --dense is written dense from the CSR the streaming transpose makes, instead of silently staying CSR (F100). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er (F112) Planning unpacked the archive beside the output before anything had made the output's folder, so converting into a new folder failed with a bare Errno naming a temporary path. The tables are now unpacked in the nearest folder that exists, and a folder that cannot take them is named. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… or --scale (F066) export image clipped every value from 255 up to 255, so a 16-bit image came back as [0, 255, 255, ...]. One channel of uint16, or of whole numbers to 65535, is now written as a 16-bit PNG or TIFF; whole numbers from 0 to 255 stay 8-bit whatever their dtype. Values an image cannot keep (negative, too large, 16-bit colour or 16-bit JPEG, not finite) are refused with their range, and the new --scale stretches them onto 0-255. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ocks Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. |
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
![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: 4a54152d78
ℹ️ 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".
| cols = np.asarray(csr["indices"][start:end], dtype=np.int64) | ||
| values = np.asarray(csr["data"][start:end]) | ||
| rows = np.repeat(np.arange(len(block)), np.diff(indptr[cells.start : cells.stop + 1])) |
There was a problem hiding this comment.
When --dense is used on a sparse imputed matrix whose feature axis is split into multiple blocks, every feature block rereads and allocates arrays for all nonzeros in the selected cells. A single very wide, dense sparse row can therefore allocate far more than BLOCK_ENTRIES, and the same row is reread once per feature block, making runtime approach quadratic as width grows. Slice or stream each row's entries once so both allocation and reads remain block-bounded.
AGENTS.md reference: AGENTS.md:L6-L10
Useful? React with 👍 / 👎.
| if hi <= 255: | ||
| return arr.astype(np.uint8) |
There was a problem hiding this comment.
For finite floats between 1 and 255, this branch casts directly to uint8 without checking that the values are whole numbers. An image containing values such as 1.9 and 254.8 is therefore silently changed to 1 and 254, despite the new contract that only whole numbers in this range are preserved and unrepresentable values are refused unless --scale is requested.
Useful? React with 👍 / 👎.
| def _stretch(arr: np.ndarray) -> np.ndarray: | ||
| """Values stretched onto 0-255: the smallest finite one to 0, the | ||
| largest to 255; any that is not finite to 0.""" | ||
| values = arr.astype(np.float64) |
There was a problem hiding this comment.
Converting integer pixels to float64 before finding the range loses distinctions above 2^53. For example, a uint64 image containing 2**63 and 2**63 + 1 makes both values equal, so hi == lo and --scale writes both as zero instead of mapping the minimum to 0 and maximum to 255. Compute the integer range before conversion, or subtract the integer minimum before converting.
Useful? React with 👍 / 👎.
Pickle reading hardened further after the independent review (F038, security). A sparse matrix's shape is read as plain integers and its index arrays must be scipy's own dtypes; a table's columns must hold as many rows as its index and each column once. Writing a sparse imputed matrix densely reads each block of cells' entries once, not once per run of features. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Test Results (py3.13)3 672 tests 3 658 ✅ 13m 10s ⏱️ Results for commit 53a91d8. ♻️ This comment has been updated with latest results. |
Test Results (py3.12)3 672 tests 3 658 ✅ 8m 46s ⏱️ Results for commit 53a91d8. ♻️ This comment has been updated with latest results. |
Review nit: a pickle cut short, or one whose contents pandas itself refuses, failed with the library's bare error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes #101. Part of #87. For 0.17.6: merge after 0.17.5 is tagged.
What changed
--denseon an imputed pickle chunks X by a fixed number of entries (BLOCK_ENTRIES, about a million: a block of cells, or a run of one cell's features when a cell has more), not by (4096 cells, every feature), so the chunk no longer grows with the features and HDF5 no longer refuses it past 262,144 float32 features.np.nonzeroallocates no longer grows with the features (it was 4096 cells x every feature, about 9x the block's bytes).--denseon a sparsemtx(gene activity) is honoured: written dense a block of cells at a time from the CSR the streaming transpose puts on disk, instead of silently staying CSR.analysis.tar.gzconverts into an output folder that does not exist yet: the archive is unpacked in the nearest existing folder of the output's, and one that cannot take it is named instead of a bare Errno.export imagekeeps the values: one channel of uint16 (or whole numbers to 65,535) as a 16-bit PNG or TIFF, whole numbers 0-255 as 8-bit whatever their dtype. Values an image cannot keep (negative, too large, 16-bit colour or 16-bit JPEG, not finite) are refused with their range instead of clipped; the new--scalestretches them onto 0-255.Also: pickles that pandas 1.5 writes with
pd.NAin a nullable text column, and old-protocol pickles of empty arrays (bytes()), now read; both were refused before.Files outside B8c's list
src/adata/cli.py: the--scaleoption and help text ofexport image(F066), about 12 lines.src/adata/commands/export.py: passesscalethrough (3 lines).tests/test_performance.pyis untouched (the F025 guard is aperf-marked test intests/test_cistopic.py, usingtests/perf_counters.py).docs/reference/commands.md,docs/guides/inspect-export.md:export image's values and--scale.How it was tested
tests/test_pickled.py(new): the F038, F040 and F140 tests (details after release) fail on main and pass here; a pickle of every protocol 0-5, and numpy scalars, big-endian, Fortran, text, empty and object arrays, nullable columns, MultiIndex and sets, read equal to numpy's and pandas' own reading.pandas.read_pickle; the integration teststest_a_pandas_15_cistopic_pickle_*(pandas 1.5 writes and checks with anndata 0.10.9) pass.perfguard counts the bytes allocated while X is written (tracemalloc, no clock) and requires them flat as features grow 64x (1.12x and 1.28x now; 47x and 51x on main).new/deeper/out.{h5ad,zarr}read back by anndata with the analysis columns, nothing left behind; an unwritable folder is named. The Xenium scanpy integration tests pass.--scalematches the linear stretch; the CLI flag is tested.COLUMNS=80 uv run pytest -m "not integration" --cov=adata --cov-fail-under=90: 3660 passed, 7 skipped, 4 xfailed, coverage 95.69% (at 65d9d2d, after merging main with 0.17.5 and B6a; the later commits are a one-line message fix and main's benchmark pages, after which the batch's own suites and the docs test pass: 680 passed);uv run pytest tests/test_external_readers.py -m integration -k "cistopic or xenium": 4 passed.Changelog entry
Fixed
F038, F040, F140 (security). Reading a pycisTopic pickle checks what it
hands numpy, scipy and pandas, and refuses a pickle that fails before
anything is written. Pickles are trusted input: read them only from a
source you would trust with the data. Dates and times are no longer read
from them; pycisTopic's objects hold none. Pickles that pandas 1.5 writes
with missing values in a nullable text column now read.
(#101).
--densechunked X as 4096 cells by every feature, which HDF5 refused past 262,144
float32 features, and CSR output held 4096 cells' coordinates at once,
about 50 GB at 500,000 regions. Both now work about a million entries at
a time, and a dense X is chunked the same way.
--denseis honoured for gene activity(#101). An imputed
object holding a sparse matrix was written as CSR although
--densewasgiven and the summary said dense.
(#101). With only
analysis.tar.gz, converting to an output whose folder did not exist yetfailed with a bare "No such file or directory" naming a temporary path.
export imagekeeps 16-bit images(#101). Every value
from 255 up was clipped to 255, silently. One channel of 16 bits is now
written as a 16-bit PNG or TIFF, and values an image cannot keep are
refused, naming their range; the new
--scalestretches them onto 0-255.Independent review
A fresh subagent with none of this context got the diff, the audit entries, D3 and AGENTS.md, and tried to get crafted pickles past the checks, checked the memory bounds and the repo rules.
tests/test_pickled.py; the reviewer's own reproductions are refused or convert cleanly on the new head.🤖 Generated with Claude Code