Skip to content

Add return_sig and decouple significance compute from display#134

Open
lshpaner wants to merge 2 commits into
mainfrom
upd_corr_matrix_LS
Open

Add return_sig and decouple significance compute from display#134
lshpaner wants to merge 2 commits into
mainfrom
upd_corr_matrix_LS

Conversation

@lshpaner

@lshpaner lshpaner commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Extends flex_corr_matrix with p-value return support and fixes two significance-related behavior bugs. All changes are backward compatible at default arguments.

Changes

  • Add return_sig to return the pairwise p-value matrix as a DataFrame, mirroring return_corr. When both return_corr and return_sig are set, returns a (corr_matrix, pval_matrix) tuple. Return annotation loosened to Optional[Union[pd.DataFrame, Tuple[pd.DataFrame, pd.DataFrame]]].
  • Fix _stars significance gate. The star gate was hardcoded at p >= 0.05, so significance_level had no effect in "stars" mode despite the docstring claiming otherwise. Now gates on significance_level, so stars and mask share one threshold. Identical output at the 0.05 default.
  • Decouple p-value computation from display. Introduced a _need_pvalues trigger (show_significance or filter_significance is not None or return_sig) so p-values are computed whenever any consumer needs them, while the stars/mask overlay stays gated on show_significance.
  • Remove silent show_significance auto-enable. filter_significance no longer overrides an explicit show_significance=False. It now filters variables without forcing the overlay, so filter_significance=0.05, show_significance=False produces a plain filtered heatmap.
  • Plot-suppression gate now covers both return flags: return_corr or return_sig suppresses the plot by default; show_plot=True re-enables.
  • Docstring reconciled: new return_sig entry, corrected significance_level / filter_significance / show_plot descriptions, updated Returns for the three-way return.

Add p_adjust, return_n, show_n to flex_corr_matrix

  • p_adjust: bonferroni / fdr_bh, corrected over unique pairs (m = k(k-1)/2), applied before stars, mask, filter_significance and return_sig consume p-values
  • return_n: pairwise sample-size matrix; n_format; toggles counts vs percent of frame
  • show_n: plots the n matrix as a coverage heatmap; mutually exclusive with show_significance
  • cmap/vmin/vmax/cbar_label now resolve by mode
  • Fix: filter_significance dropped the -0.00 zeroing
  • 40 tests, all mutation-checked; restores the pulled _stars significance_level test via gcf().axes

Tests

Added coverage for return_sig (single return, tuple return, plot suppression, diagonal, filter/threshold reflection). Note: the _stars-gate regression test was pulled pending a fix to axis-annotation readback, so the gate behavior is currently unpinned.

_________ coverage: platform linux, python 3.12.7-final-0 _________

Name                                     Stmts   Miss  Cover
------------------------------------------------------------
src/eda_toolkit/__init__.py                 19      2    89%
src/eda_toolkit/_data_manager_utils.py      28      0   100%
src/eda_toolkit/_plot_utils.py             185     23    88%
src/eda_toolkit/art.py                      54      3    94%
src/eda_toolkit/data_manager.py            846     84    90%
src/eda_toolkit/plots.py                  1314    236    82%
------------------------------------------------------------
TOTAL                                     2446    348    86%

@lshpaner lshpaner changed the title Add return_sig and decouple significance compute from display Add return_sig and decouple significance compute from display Jul 3, 2026

This comment was marked as resolved.

@lshpaner lshpaner requested review from Oscar-Gil-Data and removed request for Oscar-Gil-Data July 14, 2026 22:44
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