Skip to content

Pna 2699 - #386

Open
elhb wants to merge 51 commits into
devfrom
pna-2699
Open

Pna 2699#386
elhb wants to merge 51 commits into
devfrom
pna-2699

Conversation

@elhb

@elhb elhb commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Description

Add typed panel combination system for PNA.

Adds distinct panel kinds (base / addon / sample-hashing) driven by metadata
panel_type, plus PNAAntibodyPanelCombination for samples that use several
panels together. Shared marker helpers live on PNAPanel.

PNAPanel (ABC)
├── PartialPNAAntibodyPanel
│   ├── PNABasePanel
│   ├── PNAAddonPanel
│   └── PNASampleHashingPanel
└── PNAAntibodyPanelCombination
  • Replaces the single PNAAntibodyPanel type with the hierarchy above.
    PNAAntibodyPanel remains a deprecated alias of PartialPNAAntibodyPanel
    (DeprecationWarning on access; removed in a future release).
  • Public exports from pixelator.pna.config include PNAPanel,
    PartialPNAAntibodyPanel, typed subclasses, PanelType,
    PNAAntibodyPanelCombination, and panel_from_* / load_antibody_panel.
  • Factories (panel_from_csv, panel_from_adata, panel_from_pxl_dataset,
    load_antibody_panel) return a typed panel or a combination from metadata.
  • Combinations are built from member panels:
    PNAAntibodyPanelCombination(panel) /
    PNAAntibodyPanelCombination([p1, p2, ...]).
  • CLI accepts repeated --panel arguments; AnnData / pxl round-trips carry
    multi-panel metadata and can bump compatible patch versions.
  • New AnnData / .pxl files store panels under num_partial_panels +
    panel_metadata__{i} / panel_df__{i} (even for a single panel). Legacy
    panel_metadata + panel_columns files remain readable. Patch bumps via
    PNAAntibodyPanelDiff.upgrade_adata migrate legacy uns to the multi-panel
    layout (i.e. backwards compatability for bumping patch versions of old pxl files).
  • sample_hashing comparisons accept both bool columns (CSV parse) and legacy
    yes/no strings via sample_hashing_mask.
  • Docs: panel API docstrings and an AutoAPI class template for inheritance
    rendering.

Fixes: PNA-2699

THIS PR DOES NOT CHANGE ANY PANELS (panel CSVs are unchanged; typed
panel_type only applies when metadata sets it).

Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

API impact vs dev:

  • Prefer PartialPNAAntibodyPanel (or a typed subclass) /
    PNAAntibodyPanelCombination / PNAPanel instead of PNAAntibodyPanel.
    Existing PNAAntibodyPanel imports still work but warn.
  • load_antibody_panel returns PNAAntibodyPanelCombination.
  • Pass panels into PNAAntibodyPanelCombination(...), not a raw dataframe and
    metadata.
  • New AnnData / .pxl files store panels under num_partial_panels +
    panel_metadata__{i} / panel_df__{i} (even for a single panel). Legacy
    panel_metadata + panel_columns files are still readable, but are no
    longer written (patch bumps migrate them to the new layout).

How Has This Been Tested?

  • task test
  • Focused coverage for legacy AnnData panel round-trip and
    upgrade_adata migration to multi-panel uns
  • Docs build for AutoAPI inheritance rendering where applicable
  • nf-test test tests/proxiome_v2.nf.test --profile=+docker --verbose ran on the container built in this PR finished sucesfully.

PR checklist:

  • This comment contains a description of changes (with reason).
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • If a new tool or package is included, I have updated dependencies in pyproject.toml and cited it properly
  • I have checked my code and documentation and corrected any misspellings
  • I have documented any significant changes to the code in CHANGELOG.md

Note

Medium Risk
Refactors the core panel type used across the PNA pipeline and changes AnnData write format, though legacy reads and a deprecated import path limit breakage; incorrect multi-panel or hashing handling could affect demux and sample calling.

Overview
Introduces a typed PNA panel model so one sample can use several panels (base, addon, sample-hashing) together. The monolithic PNAAntibodyPanel is split into PNAPanelPartialPNAAntibodyPanel and typed subclasses (PNABasePanel, PNAAddonPanel, PNASampleHashingPanel), with PNAAntibodyPanelCombination as the object passed through demux, collapse, graph, sample calling, and AnnData/pxl I/O. CSV metadata may set panel_type; factories (panel_from_csv, panel_from_adata, load_antibody_panel, etc.) dispatch to the right class or build a combination.

API and CLI: load_antibody_panel always returns a PNAAntibodyPanelCombination (even for one panel). CLI --panel is repeatable for multi-panel runs. PNAAntibodyPanel remains as a deprecated alias of PartialPNAAntibodyPanel (DeprecationWarning on access).

Persistence: New AnnData/pxl files store num_partial_panels, panel_metadata__{i}, and panel_df__{i} instead of legacy panel_metadata + panel_columns; readers still accept the old shape, and PNAAntibodyPanelDiff.upgrade_adata migrates legacy uns on patch bumps.

Bugfix: Combining a base panel without sample_hashing with a hashing panel no longer drops hashing markers—sample_hashing_mask normalizes bool/string/float-upcast values after pd.concat.

Docs add an AutoAPI class.rst override so inheritance Bases: links resolve on own-page class docs.

Reviewed by Cursor Bugbot for commit 4a630f5. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor

cursor Bot commented Aug 20, 2026

Copy link
Copy Markdown

Current version of PR was reviewed by /review-bugbot with additional instructions on Aug 20, 12:11 GMT+2. It flagged 2 findings.

Show 2 findings

1. Legacy patch bump skips uns

src/pixelator/pna/config/panel.py:906 · Medium Severity

PNAAntibodyPanelDiff.upgrade_adata only refreshes panel_metadata__* / panel_df__* when num_partial_panels is present. Legacy AnnData that still uses a single panel_metadata entry (with panel_columns) gets an updated var table but stale uns panel metadata, unlike the previous implementation.

2. Deserialization mutates adata uns

src/pixelator/common/config/panel.py:125 · Medium Severity

AntibodyPanelMetadata._deserialize_from_adata_key modifies the dict stored in adata.uns in place when converting panel_type strings to PanelType enums, instead of validating a copy.

Bugbot on commit e446f40 is skipped.

Comment thread src/pixelator/pna/config/panel.py Outdated
Comment thread src/pixelator/pna/config/panel.py Outdated
Comment thread src/pixelator/pna/config/panel.py Outdated
Comment thread src/pixelator/pna/config/panel.py Outdated
…n sync with df

Co-authored-by: Cursor <cursoragent@cursor.com>
@elhb
elhb requested review from Aratz and johandahlberg August 21, 2026 09:20
Split config/panel.py over multiple files
Comment thread src/pixelator/pna/config/panel/utils.py Outdated
pd.concat upcast bool flags to float (True → 1.0), and sample_hashing_mask
did not treat "1.0" as hashing, so sample calling saw an empty hashing set.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread src/pixelator/pna/config/panel/combination.py Outdated
Comment thread src/pixelator/pna/config/panel/combination.py Outdated
Comment thread src/pixelator/pna/config/panel/partial.py Outdated
…type.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread src/pixelator/pna/analysis/denoise.py
Multi-panel names are stored as "base + hash", so splitting on "+" without
stripping left padded fragments that load_antibody_panel could not resolve.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4a630f5. Configure here.

nodes_to_remove: pl.DataFrame,
hash_info: pl.DataFrame,
panel: PNAAntibodyPanel,
panel: PNAAntibodyPanelCombination,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dehashed panel metadata stays hashed

High Severity

Sample calling builds AnnData with the full panel (including hashing) via pna_edgelist_to_anndata, which stores independent panel_df__{i} snapshots in uns, then drops hashing markers from var without updating those snapshots. Unlike the legacy path that rebuilt the panel from var, PNAAntibodyPanelCombination.from_adata now restores hashing members from uns, so dehashed .pxl files keep hashing antibodies in recovered panel metadata.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4a630f5. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants