Skip to content

Return NaN instead of crashing when every EMD stratum is too small - #131

Merged
ghar1821 merged 5 commits into
mainfrom
fix-emd-empty-strata
Jul 30, 2026
Merged

Return NaN instead of crashing when every EMD stratum is too small#131
ghar1821 merged 5 commits into
mainfrom
fix-emd-empty-strata

Conversation

@rcannood

Copy link
Copy Markdown
Member

Describe your changes

Both EMD paths skip a stratum when either side has fewer than 50 cells, and both then call pd.concat() on the collected results. If every stratum is skipped, that list is empty and pd.concat() raises ValueError: No objects to concatenate, so the whole metric component dies instead of reporting that it couldn't be computed.

The vertical path already handles the neighbouring case gracefully -- if there aren't at least 2 samples per group it returns np.nan -- so this PR extends the same treatment to the empty-results case in both paths:

  • get_vert_emd_for_integrated_adata() returns np.nan when no sample combination and cell type had enough cells.
  • calculate_horizontal_emd() returns np.nan for both its keys when no donor and cell type had enough cells.

Also two small things in the same block:

  • The "some cell types are in split 1 but not in split 2" warning was gated on len(cell_type_not_in_both) > 1, so it stayed quiet in the most common case of exactly one missing cell type. Changed to > 0.
  • That warning's message ended in ]n rather than a newline, and joined the cell type names without a separator.

Note that with errorStrategy set to ignore after retries, a crash here currently looks the same as a missing score in the output -- which is part of why it is worth turning into an explicit NaN.

Found while reviewing the task ahead of the next full benchmark run -- see also the sibling PRs.

Checklist before requesting a review

  • I have performed a self-review of my code

  • Check the correct box. Does this PR contain:

    • Breaking changes
    • New functionality
    • Major changes
    • Minor changes
    • Bug fixes
  • Proposed changes are described in the CHANGELOG.md

  • CI Tests succeed and look good!

* Guard the pd.concat() calls in both the horizontal and vertical path
* Warn as soon as one cell type is missing from a split, not two
* Fix the newline in that warning
@rcannood
rcannood requested review from LuLeom and ghar1821 July 28, 2026 11:19
@ghar1821

Copy link
Copy Markdown
Contributor

Not fixing the emd crashing on pd.concat() when every donor/sample combination and cell type falls below the 50 cell threshold, because we would have detected this prior to running any methods or metrics, and we would not have included the dataset if this is the case (PR #131).

@ghar1821
ghar1821 merged commit d803136 into main Jul 30, 2026
2 checks passed
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.

3 participants