You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: warn visibly about deprecated pairplot kwargs, drop a stray label under the legend - #2160
Passing old-style keyword arguments such as legend=True or samples_labels=[...] to pairplot or marginal_plot sends the whole call to the pre-v0.23 implementation (pairplot_dep, marginal_plot_dep). That path has three problems:
Nobody sees the deprecation warning. Since Refactor Plotting functions #1631 (v0.26), the DeprecationWarning is raised inside a helper with stacklevel=2. It points into sbi/analysis/plot.py, so Python's default filters hide it, in scripts and in notebooks. It never named a removal version, and since Refactor Plotting functions #1631 it no longer lists the keys either.
The old implementation silently drops newer arguments. It never receives lower, diag_kwargs, upper_kwargs, lower_kwargs, fig_kwargs, discrete_indices or the names of LabeledSamples. Advanced tutorial 19 mixes both styles in four cells, so it draws its contours at the old default of 68 % instead of the requested 95 % and 99 %, and ignores bins=100.
PlotOffDiagOptions is still exported, but no style uses it.upper="plot" raises, and passing it as upper_kwargs crashes scatter and contour.
Moving tutorial 19 to the current implementation exposed a bug there, too. With fig_kwargs=dict(legend=True) and an empty lower triangle, the legend goes into the bottom-left cell, and that cell's x-label (e.g. "dim 1") is drawn under it. The cell got its label before it was turned off; _place_legend_in_empty_cell (#1830) turns it back on and hides its ticks and spines, but not its labels. Tutorial 17 shows the same stray label.
This PR:
raises a FutureWarning at the caller's line. It names the keys, says they will be removed in v0.29.0, and says where they go instead. If newer arguments were passed too, it names the ones that are ignored;
warns when pairplot_dep or marginal_plot_dep is called directly (Refactor pairplot function #1425 mentions doing that). They become thin wrappers around the unchanged bodies, now _pairplot_dep and _marginal_plot_dep. pairplot and marginal_plot call the bodies, so each call warns once;
warns when PlotOffDiagOptions is created. Importing it stays silent;
clears the axis labels of the cell that holds the legend;
moves tutorial 19's options into fig_kwargs, drops PlotOffDiagOptions from tutorial 17's list, and reruns both tutorials.
The old implementation itself doesn't change: calls that use it draw the same figures as before.
Evidence
What a plain script shows under Python's default warning filters:
script.py:22: FutureWarning: `pairplot` got the deprecated keyword arguments ['legend'].
They are deprecated since sbi v0.23.0 and will be removed in v0.29.0. Pass `FigOptions`
fields in `fig_kwargs` and plot styles in the `*_kwargs` arguments (see
`sbi.analysis.plotting_classes`).
The legend fix changes only figures with a legend in the bottom-left cell. I rendered 9 cases: the legend in either empty triangle or on the diagonal, with and without background colours, no legend, the old implementation and marginal_plot. The 3 with a legend in the bottom-left cell change; the other 6 are pixel-identical to main.
Tutorials 17 and 19, rerun in full: tutorial 19's four cells now go through the current implementation. Its MAP figure shows the requested 95 % contour with 100 KDE bins, and its i.i.d. and guidance figures show the requested 99 % contours. The legend figures of both tutorials no longer show the stray label. Neither notebook has a seed, so the other outputs and the epoch counts change too. Outputs are stored as in the other docs notebooks: plots and printed output only (tests/strip_notebook_outputs.py), with consecutive prints merged and no execution timestamps.
Tests:
test_deprecated_plotting_warns_once_at_the_caller (8 cases): each deprecated
entry point warns exactly once, names v0.29.0 and points at the caller all 8 fail on main
test_pairplot_dep: now expects the FutureWarning fails on main
test_pairplot_legend_cell_has_no_axis_labels (2 cases): the cell that holds
the legend has no axis labels the bottom-left case fails on main
tests/plot_test.py and tests/sbc_test.py::test_sbc_plotting pass (574 passed, 9 expected failures). None of the new warnings shows up in the pytest warnings summary, so sbi never triggers them itself. pyright sbi and pre-commit on the changed files pass. I did not run the full test suite locally.
Merge Danger
Door: two-way. The only signature change: pairplot_dep and marginal_plot_dep now take *args, **kwargs and forward them unchanged.
Blast Radius:
Code that uses the old keyword arguments, pairplot_dep, marginal_plot_dep or PlotOffDiagOptions now shows a FutureWarning. What it draws doesn't change.
Figures with a legend in the bottom-left cell no longer show that cell's x-label.
Tutorials 17 and 19 are rerun, so their figures change. Tutorial 19's now show what its code asks for.
Timing: this should ship in v0.28 so that the old path can go in v0.29. If it misses v0.28, the version numbers in the four warning texts and in the test need a bump.
For review: is v0.29 the right removal version? The old keyword arguments warned visibly in v0.23 to v0.25. pairplot_dep, marginal_plot_dep and PlotOffDiagOptions never warned before, so v0.29 gives them one release of notice. Moving any of them to v0.30 only changes the four warning texts and the test.
To remove in v0.29:
the **kwargs parameters and branches of pairplot and marginal_plot, and _warn_deprecated_kwargs;
pairplot_dep, marginal_plot_dep, their bodies _pairplot_dep and _marginal_plot_dep, and get_diag_func, which only those bodies use;
PlotOffDiagOptions and its export, the "plot" branches in get_offdiag_funcs and get_default_offdiag_kwargs, plt_plot_2d, and the "hist2d", "kde2d" and "contourf" aliases there. pairplot rejects all of these style names before they are reached.
_get_default_opts, _arrange_plots and update stay: conditional_pairplot and conditional_marginal_plot use them.
Related: #1084 added the old path, #1631 moved the warning into the helper, and #1830 added the legend placement. #2111 changes the sampling that tutorial 19's cells 26 and 28 show; whichever of the two merges second should rerun tutorial 19. #2154 edits sbi/utils/plotting_helpers.py, which this PR doesn't touch, and adds a test to tests/plot_test.py; the two merge without conflicts.
I used Claude Code for the analysis, the implementation, the tests and the tutorial reruns.
Passing old-style keyword arguments to `pairplot` or `marginal_plot` sends
the call to the pre-v0.23 implementation. Since sbi-dev#1631 the
DeprecationWarning pointed into sbi, so Python's default filters hid it. It
named no removal version, and the old implementation silently dropped the
newer arguments passed with it.
Raise a FutureWarning at the caller's line instead. It names the keys,
v0.29.0 and where the keys go, and lists the newer arguments that the call
ignores. `pairplot_dep`, `marginal_plot_dep` and `PlotOffDiagOptions` warn
too. The old implementation itself is unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cells 16, 19, 26 and 28 of advanced tutorial 19 passed `legend`,
`samples_labels`, `points_labels`, `points_colors` and `points_offdiag`
directly to `pairplot`. That sent them to the old implementation, which
ignored their `diag_kwargs` and `upper_kwargs`. Pass these options in
`fig_kwargs` instead, so that the requested contour levels and KDE bins
are drawn.
Also drop the deprecated `PlotOffDiagOptions` from tutorial 17's list of
option classes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rerun advanced tutorial 19 so that its stored figures show what the cells
ask for: the 95 % contour and 100 KDE bins in the MAP figure, and the
99 % contours in the i.i.d. and guidance figures. Outputs are stored as
in the other docs notebooks: plots and printed output only, with
consecutive prints merged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Plotting functions now warn when callers use deprecated keyword arguments or entry points. PlotOffDiagOptions also emits a deprecation warning. Legend cells no longer retain axis labels.
Changes: Plotting API updates
Layer / File(s)
Summary
Deprecation warnings and legacy routing sbi/analysis/plot.py, sbi/analysis/plotting_classes.py, tests/plot_test.py
marginal_plot and pairplot report deprecated keyword arguments and ignored modern options before forwarding to private implementations. Deprecated entry points and PlotOffDiagOptions emit FutureWarnings. Documentation and tests cover the deprecation details.
Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentati…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Title check
The title clearly names the deprecated pairplot keyword warnings and the stray legend-cell label fix, which are central changes in the PR.
Description check
The description explains the changes, motivation, compatibility impact, tests, related issues, and open review question. It does not use the template’s exact section headings or include the checklist,…
Full details: Docstring Coverage
Explanation
Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 8 functions across 3 files.
Full details: Description check
Explanation
The description explains the changes, motivation, compatibility impact, tests, related issues, and open review question. It does not use the template’s exact section headings or include the checklist, but it provides enough substantive information to review the PR.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Review coverage is incomplete: 2 files could not be fully reviewed. Findings from completed review steps are included; see review info for details.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.
With `fig_kwargs=dict(legend=True)` and an empty lower triangle, pairplot
puts the legend into the bottom-left cell. That cell got its x-label (e.g.
"dim 1") before it was turned off, and `_place_legend_in_empty_cell` (sbi-dev#1830)
turned it back on, hiding only its ticks and spines. Clear the labels too.
Only figures with a legend in the bottom-left cell change. The old
implementation never labels that cell, so it draws the same as before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Their legend figures no longer show the stray x-label under the legend.
Outputs are stored as in the other docs notebooks: plots and printed
output only, consecutive prints merged, and no execution timestamps. This
also drops the timestamps that the previous rerun of tutorial 19 stored.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dgedon
changed the title
fix: warn visibly about deprecated pairplot keyword arguments, removed in v0.29
fix: warn visibly about deprecated pairplot kwargs, drop a stray label under the legend
Nov 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
dgedon
changed the title
Summary
Passing old-style keyword arguments such as
legend=Trueorsamples_labels=[...]topairplotormarginal_plotsends the whole call to the pre-v0.23 implementation (pairplot_dep,marginal_plot_dep). That path has three problems:DeprecationWarningis raised inside a helper withstacklevel=2. It points intosbi/analysis/plot.py, so Python's default filters hide it, in scripts and in notebooks. It never named a removal version, and since Refactor Plotting functions #1631 it no longer lists the keys either.lower,diag_kwargs,upper_kwargs,lower_kwargs,fig_kwargs,discrete_indicesor the names ofLabeledSamples. Advanced tutorial 19 mixes both styles in four cells, so it draws its contours at the old default of 68 % instead of the requested 95 % and 99 %, and ignoresbins=100.PlotOffDiagOptionsis still exported, but no style uses it.upper="plot"raises, and passing it asupper_kwargscrashesscatterandcontour.Moving tutorial 19 to the current implementation exposed a bug there, too. With
fig_kwargs=dict(legend=True)and an empty lower triangle, the legend goes into the bottom-left cell, and that cell's x-label (e.g. "dim 1") is drawn under it. The cell got its label before it was turned off;_place_legend_in_empty_cell(#1830) turns it back on and hides its ticks and spines, but not its labels. Tutorial 17 shows the same stray label.This PR:
FutureWarningat the caller's line. It names the keys, says they will be removed in v0.29.0, and says where they go instead. If newer arguments were passed too, it names the ones that are ignored;pairplot_depormarginal_plot_depis called directly (Refactor pairplot function #1425 mentions doing that). They become thin wrappers around the unchanged bodies, now_pairplot_depand_marginal_plot_dep.pairplotandmarginal_plotcall the bodies, so each call warns once;PlotOffDiagOptionsis created. Importing it stays silent;fig_kwargs, dropsPlotOffDiagOptionsfrom tutorial 17's list, and reruns both tutorials.The old implementation itself doesn't change: calls that use it draw the same figures as before.
Evidence
What a plain script shows under Python's default warning filters:
mainpairplot(s, legend=True)FutureWarningat the caller's line (text below)pairplot(s, legend=True, diag="kde", diag_kwargs=...)diag_kwargsis droppedpairplot(LabeledSamples(s, name="a"), legend=True)'LabeledSamples names'marginal_plot(s, legend=True)FutureWarningpairplot_dep(s),marginal_plot_dep(s)FutureWarningPlotOffDiagOptions()FutureWarningpairplot(s, fig_kwargs=dict(legend=True))python -W error::FutureWarning -c "import sbi.analysis"passes, so importing stays silent.The legend fix changes only figures with a legend in the bottom-left cell. I rendered 9 cases: the legend in either empty triangle or on the diagonal, with and without background colours, no legend, the old implementation and
marginal_plot. The 3 with a legend in the bottom-left cell change; the other 6 are pixel-identical tomain.Tutorials 17 and 19, rerun in full: tutorial 19's four cells now go through the current implementation. Its MAP figure shows the requested 95 % contour with 100 KDE bins, and its i.i.d. and guidance figures show the requested 99 % contours. The legend figures of both tutorials no longer show the stray label. Neither notebook has a seed, so the other outputs and the epoch counts change too. Outputs are stored as in the other docs notebooks: plots and printed output only (
tests/strip_notebook_outputs.py), with consecutive prints merged and no execution timestamps.Tests:
tests/plot_test.pyandtests/sbc_test.py::test_sbc_plottingpass (574 passed, 9 expected failures). None of the new warnings shows up in the pytest warnings summary, so sbi never triggers them itself.pyright sbiandpre-commiton the changed files pass. I did not run the full test suite locally.Merge Danger
Door: two-way. The only signature change:
pairplot_depandmarginal_plot_depnow take*args, **kwargsand forward them unchanged.Blast Radius:
pairplot_dep,marginal_plot_deporPlotOffDiagOptionsnow shows aFutureWarning. What it draws doesn't change.Timing: this should ship in v0.28 so that the old path can go in v0.29. If it misses v0.28, the version numbers in the four warning texts and in the test need a bump.
For review: is v0.29 the right removal version? The old keyword arguments warned visibly in v0.23 to v0.25.
pairplot_dep,marginal_plot_depandPlotOffDiagOptionsnever warned before, so v0.29 gives them one release of notice. Moving any of them to v0.30 only changes the four warning texts and the test.To remove in v0.29:
**kwargsparameters and branches ofpairplotandmarginal_plot, and_warn_deprecated_kwargs;pairplot_dep,marginal_plot_dep, their bodies_pairplot_depand_marginal_plot_dep, andget_diag_func, which only those bodies use;PlotOffDiagOptionsand its export, the"plot"branches inget_offdiag_funcsandget_default_offdiag_kwargs,plt_plot_2d, and the"hist2d","kde2d"and"contourf"aliases there.pairplotrejects all of these style names before they are reached._get_default_opts,_arrange_plotsandupdatestay:conditional_pairplotandconditional_marginal_plotuse them.Related: #1084 added the old path, #1631 moved the warning into the helper, and #1830 added the legend placement. #2111 changes the sampling that tutorial 19's cells 26 and 28 show; whichever of the two merges second should rerun tutorial 19. #2154 edits
sbi/utils/plotting_helpers.py, which this PR doesn't touch, and adds a test totests/plot_test.py; the two merge without conflicts.I used Claude Code for the analysis, the implementation, the tests and the tutorial reruns.
🤖 Generated with Claude Code