bigbio / bigbio/qpx

design_matrix(fillna=None) turns a stored NULL pg intensity into 0.0; the in-memory and out-of-core pivots disagree

Open
#281 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
26
Forks
8
Avg merge
5h 13m
Merged PRs (30d)
27

Description

`Dataset.design_matrix(level="protein", fillna=None)` is documented to preserve missingness, but a `pg` row whose `label` is non-null and whose `intensity` is **NULL** comes out of it as `0.0`.

Checked against `main` @ `b361f204` (= `v1.1.2`), with pandas 3.0.1 and duckdb 1.5.5.

### What happens

`_abundance_sql("protein")` correctly preserves the NULL — it selects `p.intensity` and filters only on label match and `is_decoy` (`qpx/dataset.py:546-552`), so the row reaches pandas as `NaN`.

The in-memory pivot then aggregates it away (`qpx/dataset.py:677`):

```python
matrix = df.pivot_table(index="sample_accession", columns="feature_id",
values="intensity", aggfunc="sum")
...
if fillna is not None:
matrix = matrix.fillna(fillna)
```

pandas' `aggfunc="sum"` uses `min_count=0`, so a group whose only value is `NaN` aggregates to `0.0`. That happens before `fillna` is consulted, and the check at `:681` cannot tell that synthesized zero from a real one — so `fillna=None` cannot preserve the missingness the docstring promises at `:637-638`:

```
fillna : float | None
Value to fill missing cells. Use *None* to keep NaN.
```

Note this is not just an ordering problem: reordering the `fillna` check alone would not fix `fillna=None`, because the null is already gone.

### The two documented paths disagree

The out-of-core path (`output_path=`) uses DuckDB `PIVOT ... USING SUM(intensity)`, and DuckDB returns `NULL` for an all-NULL group. So the same input gives `0.0` in memory and `NULL` on disk. `fillna` being ignored under `output_path` is documented (`:638`); this difference in aggregation semantics is not.

```
pandas 3.0.1 : all-NaN group, aggfunc="sum" -> 0.0 (isna: False)
duckdb 1.5.5 : SUM over an all-NULL group -> NULL
```

### Why it matters

A NULL `pg.intensity` beside a populated label means "there is evidence for this group here, but it was not quantified for this label". Turning it into `0.0` presents it as a measurement of zero, which then participates in ratios, log transforms and normalisation as data. It is the same failure mode as the `observed_mz = 0.0` sentinel removed in #244 — a sentinel that looks like a real value, where NULL would have propagated as "unknown".

The `fillna=0.0` default is not in question: asking for zeros and getting zeros is fine. The problem is that `fillna=None` cannot opt out.

### Reachability

This is not hypothetical. `openms_consensus` emits exactly this shape — `_rollup` returns `None` when a group has no unique-peptide signal in a unit+label, and that value is written with the populated label (`qpx/converters/openms_consensus/pg_adapter.py:345`). `PgWriter` deliberately preserves it (`qpx/writers/pg.py:43-52`).

### Test coverage

`tests/integration/test_dataset.py:478` exercises `fillna=None`, but only for an **absent** sample/protein combination — a coordinate missing from the pivot entirely, which does stay `NaN`. No test covers a present row carrying a NULL intensity, which is the case that collapses.

### Repro

```python
import pandas as pd, numpy as np
df = pd.DataFrame({"sample_accession": ["S1"], "feature_id": ["P1"], "intensity": [np.nan]})
m = df.pivot_table(index="sample_accession", columns="feature_id",
values="intensity", aggfunc="sum")
print(repr(m.loc["S1", "P1"]), pd.isna(m.loc["S1", "P1"])) # 0.0 False
```

End to end, a single pg row with `label='LFQ'`, `intensity=NULL` and a matching `run.samples[]` entry yields `design_matrix(level="protein", fillna=None).loc["S1", "P1"] == 0.0`, while the same base query under the DuckDB pivot yields `NULL`.

### Possible directions

Not a patch proposal, just the options I can see: aggregate with `min_count=1` so an all-null group stays `NaN`; or aggregate in DuckDB for both paths so the two agree; or, if the current behaviour is intended, document that `fillna=None` preserves only *absent coordinates* and not *stored nulls*, and say what a consumer should use instead to distinguish them.

Contributor guide

Open the contributing guide

Research direction

Start in qpx/dataset.py at _abundance_sql and the in-memory pivot around lines 637-681, then compare the output_path pivot behavior. Run tests/integration/test_dataset.py around line 478 and reproduce the present-row NULL case. Done means fillna=None preserves a stored NULL and the in-memory and out-of-core paths agree, with regression coverage for that case.

Written by the indexing model from the issue text.

Assessment

Tech stack
pandas, python, sql
Domain
data, databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.