fix: do not force matplotlib Agg backend on import#53
Conversation
Importing the package called matplotlib.use("Agg") at module level,
which globally hijacked the backend for the whole Python process and
broke interactive use (IPython/Jupyter) and the show=True option.
Remove the global backend override and let matplotlib auto-select the
appropriate backend (it still falls back to Agg in headless/CI
environments, so file saving via plot_file is unaffected).
Adds a regression test verifying that importing the package preserves
the user's chosen backend.
Refs #52
Reviewer's GuideRemoves the forced global matplotlib Agg backend selection on package import and adds a regression test to ensure the user’s chosen backend is preserved when importing the library. Sequence diagram for matplotlib backend behavior on pyoctaveband importsequenceDiagram
actor User
participant IPython
participant matplotlib
participant pyoctaveband
User->>IPython: run %matplotlib svg
IPython->>matplotlib: use("svg")
User->>IPython: import pyoctaveband
IPython->>pyoctaveband: import
alt before_PR
pyoctaveband->>matplotlib: use("Agg")
matplotlib-->>IPython: backend Agg
else after_PR
pyoctaveband-->>IPython: no backend change
matplotlib-->>IPython: backend svg
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
More reviews will be available in 2 minutes and 16 seconds. Learn how PR review limits work. To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR removes matplotlib import and backend configuration from package initialization. It adds a subprocess-based test to ensure importing pyoctaveband does not change the user's matplotlib backend. The package version is bumped to 1.2.3. ChangesMatplotlib Import Side Effects
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the global configuration of the matplotlib backend (matplotlib.use("Agg")) from the package initialization to prevent overriding the user's chosen backend, and adds a test to verify this behavior. The review feedback suggests propagating the parent process's sys.path via the PYTHONPATH environment variable to the test's subprocess to ensure the test is robust and does not fail with a ModuleNotFoundError in environments where the package is not pre-installed.
| import subprocess | ||
| import sys | ||
|
|
||
|
|
||
| def test_import_does_not_override_matplotlib_backend() -> None: | ||
| """Importing pyoctaveband must preserve the user's chosen backend.""" | ||
| code = ( | ||
| "import matplotlib\n" | ||
| # Pick an explicit, always-available backend the user might have set. | ||
| "matplotlib.use('svg')\n" | ||
| "before = matplotlib.get_backend()\n" | ||
| "import pyoctaveband\n" | ||
| "after = matplotlib.get_backend()\n" | ||
| "assert before == after, f'backend changed: {before!r} -> {after!r}'\n" | ||
| ) | ||
| result = subprocess.run( | ||
| [sys.executable, "-c", code], | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| assert result.returncode == 0, result.stderr |
There was a problem hiding this comment.
The subprocess is executed without propagating the current sys.path or setting PYTHONPATH. If the package is not installed in the environment (for example, when running tests locally via pytest where the path is dynamically added to sys.path), the subprocess will fail with ModuleNotFoundError: No module named 'pyoctaveband'.
To ensure the test is robust across different test runners and environments, pass the parent process's sys.path via the PYTHONPATH environment variable to the subprocess.
import os
import subprocess
import sys
def test_import_does_not_override_matplotlib_backend() -> None:
"""Importing pyoctaveband must preserve the user's chosen backend."""
code = (
"import matplotlib\n"
# Pick an explicit, always-available backend the user might have set.
"matplotlib.use('svg')\n"
"before = matplotlib.get_backend()\n"
"import pyoctaveband\n"
"after = matplotlib.get_backend()\n"
"assert before == after, f'backend changed: {before!r} -> {after!r}'\n"
)
env = os.environ.copy()
env["PYTHONPATH"] = os.path.pathsep.join(sys.path)
result = subprocess.run(
[sys.executable, "-c", code],
capture_output=True,
text=True,
env=env,
)
assert result.returncode == 0, result.stderr![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_matplotlib_backend.py`:
- Around line 26-30: The subprocess.run call that assigns to result is missing a
timeout and can hang; add a timeout argument (e.g., timeout=10) to the
subprocess.run invocation in tests/test_matplotlib_backend.py and wrap the call
in a try/except catching subprocess.TimeoutExpired to fail the test with a clear
message (use the same variable name result and reference
subprocess.TimeoutExpired in the handler) so the test won't hang indefinitely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 27ba4be7-99b0-4d57-b446-defa5f667290
📒 Files selected for processing (2)
src/pyoctaveband/__init__.pytests/test_matplotlib_backend.py
💤 Files with no reviewable changes (1)
- src/pyoctaveband/init.py
CI Results ❌Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-30 09:43:57 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
CI Results 🚀Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-30 09:44:50 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
Propagate the parent's sys.path to the subprocess so it can import the package when running without an installed build, and add a timeout so a hung import cannot block the suite indefinitely. Addresses review feedback on PR #53.
CI Results ❌Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-30 09:51:20 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
Removing the forced backend (issue #52) exposed that the test suite never selected its own matplotlib backend. On Windows runners matplotlib defaults to the interactive TkAgg backend, so plt.subplots() in the plot tests tried to create a Tk window and failed (broken Tcl/Tk on the runner). Linux/macOS happened to fall back to Agg, hiding this. Set MPLBACKEND=Agg in conftest (the consumer opting into a headless backend, mirroring scripts/benchmark_filters.py) instead of forcing it in the library. Uses setdefault so it can still be overridden locally.
|
CI Results 🚀Test Summary
Technical Benchmark Summary📊 View Benchmark DetailsPyOctaveBand: Technical Benchmark ReportGenerated: 2026-05-30 10:13:19 1. Test Signal Parameters
2. Crossover (Linkwitz-Riley)
3. Precision & Isolation
4. Performance
|
Summary
Importing the package called
matplotlib.use("Agg")at module level, which globally hijacked the matplotlib backend for the whole Python process. This broke interactive use (IPython / Jupyter) and made theshow=Trueoption a no-op.This PR removes the global backend override and lets matplotlib auto-select the appropriate backend.
Why this is safe
Aggwhen no display is available, so saving plots viaplot_file(plt.savefig) is unaffected. The full suite passes in this headless environment, confirming it.show=True: now actually displays figures in environments with a display (previously suppressed by the forcedAggbackend).Changes
src/pyoctaveband/__init__.py: removematplotlib.use("Agg")and the now-unusedimport matplotlib.tests/test_matplotlib_backend.py: regression test verifying that importing the package preserves the user's chosen backend (setssvgin a clean subprocess, imports, asserts it is unchanged).Tests
All 125 tests pass (including the new regression test).
Refs #52
Summary by Sourcery
Stop forcing a global non-interactive matplotlib backend on import so that the library respects the user’s chosen backend and works correctly in interactive environments.
Bug Fixes:
Tests:
Summary by CodeRabbit
Bug Fixes
Tests
Chores