Skip to content

Add scalene profiling wrapper as opt-in dependency group#507

Merged
satra merged 2 commits into
alphafrom
20260503-235625-scalene-profiling
May 5, 2026
Merged

Add scalene profiling wrapper as opt-in dependency group#507
satra merged 2 commits into
alphafrom
20260503-235625-scalene-profiling

Conversation

@satra

@satra satra commented May 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a Scalene-based profiling tool to senselab as an opt-in developer dependency. A single CLI wrapper (scripts/profile_with_scalene.py) accepts any .py script or .ipynb notebook, transparently converts notebooks via nbconvert (with an injected IPython stub so magic calls survive), and produces self-contained HTML or JSON reports under artifacts/scalene/.

  • Scalene + nbconvert live in a new [dependency-groups.profiling] entry in pyproject.toml. Default uv sync is unchanged — production installs do not pay the profiler cost.
  • The wrapper invokes Scalene 2.2 in two steps: scalene run produces JSON, scalene view --standalone produces a single self-contained HTML.
  • Supports path-substring scoping via --scope, --no-thirdparty, --exclude, plus --cpu-only, --gpu, --keep-intermediate, --format {html,json}.
  • 5 pytest smoke tests under src/tests/scripts/, each guarded by @pytest.mark.skipif(not scalene_available) so the existing CI suite is unaffected when the profiling group is not installed.

To use:

uv sync --group profiling
uv run python scripts/profile_with_scalene.py path/to/script.py
uv run python scripts/profile_with_scalene.py tutorials/audio/x.ipynb

Coexists with scripts/profile_imports.py (cold-start import-time profiler from a previous feature). The two are complementary: profile_imports.py measures import time, profile_with_scalene.py measures everything after imports.

Spec, plan, and CLI contract live in specs/20260503-235625-scalene-profiling/ (workspace-only, not part of this commit).

Test plan

  • uv sync (no flag) does not pull scalene
  • uv sync --group profiling installs scalene 2.2.x and nbconvert >=7
  • uv run pytest src/tests/scripts/ passes (5 tests)
  • uv run pytest -x --ignore=src/tests/scripts still passes (existing suite unaffected)
  • uv run python scripts/profile_with_scalene.py --help prints all CLI options
  • uv run python scripts/profile_with_scalene.py /tmp/tiny.py produces HTML under artifacts/scalene/
  • uv run python scripts/profile_with_scalene.py some_tutorial.ipynb converts and profiles successfully

🤖 Generated with Claude Code

Version

Published prerelease version: 1.3.1-alpha.27

Changelog

🐛 Bug Fix

  • Add scalene profiling wrapper as opt-in dependency group #507 (@satra)

Authors: 1

Adds scripts/profile_with_scalene.py: a thin CLI wrapper around Scalene
2.2's `run` and `view` subcommands. Accepts .py or .ipynb targets,
auto-converts notebooks via nbconvert (with an injected IPython stub so
magic calls survive the conversion), and writes self-contained HTML or
JSON reports under artifacts/scalene/.

Scalene + nbconvert live in a new [dependency-groups.profiling] entry
in pyproject.toml, so default `uv sync` is unaffected — production
installs do not pay the profiler's cost. To use:

  uv sync --group profiling
  uv run python scripts/profile_with_scalene.py path/to/script.py
  uv run python scripts/profile_with_scalene.py tutorials/audio/x.ipynb

Tests in src/tests/scripts/profile_with_scalene_test.py are guarded by
@pytest.mark.skipif(not scalene_available), so they silently skip on the
default install path and the existing CI suite is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@satra
satra temporarily deployed to docs-preview May 4, 2026 17:15 — with GitHub Actions Inactive

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new profiling utility, scripts/profile_with_scalene.py, which wraps Scalene to support profiling of both Python scripts and Jupyter notebooks. The changes include adding scalene and nbconvert to the project's optional dependencies and providing a comprehensive smoke test suite. Review feedback highlights two technical issues in the notebook conversion process: a potential SyntaxError caused by prepending the IPython stub before from future imports, and an ineffective module stubbing implementation that could lead to ImportError when accessing IPython submodules.

Comment thread scripts/profile_with_scalene.py Outdated
converted.rename(py_path)
converted = py_path
original = converted.read_text(encoding="utf-8")
converted.write_text(_IPYTHON_STUB + "\n" + original, encoding="utf-8")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Prepending the _IPYTHON_STUB directly to the converted notebook script will cause a SyntaxError if the notebook contains from __future__ imports (e.g., from __future__ import annotations). In Python, from __future__ statements must appear at the very beginning of the file, before any other code or imports (including the import sys inside the stub).

You should identify the end of the from __future__ block and insert the stub after it to ensure the converted script remains valid and executable.

Comment on lines +157 to +190
_IPYTHON_STUB = """\
# Auto-injected by profile_with_scalene.py — stub IPython helpers so
# notebooks converted via nbconvert can run as plain Python under Scalene.
# This is best-effort: cells whose semantics depend on real IPython behavior
# (e.g., `%matplotlib inline` actually configuring backends, `!shell-cmd`
# side effects, `display(obj)` returning a value, widget event loops) will
# silently no-op. Profile the resulting script accordingly.
import sys as _scalene_wrapper_sys
import types as _scalene_wrapper_types

class _NoOpIPython:
def __getattr__(self, name):
return self
def __call__(self, *args, **kwargs):
return None

def get_ipython(): # noqa: N802
return _NoOpIPython()

# Stub the IPython package + common submodules so `from IPython.display import display`
# and similar import statements (preserved verbatim by nbconvert) do not raise.
if "IPython" not in _scalene_wrapper_sys.modules:
_ipy = _scalene_wrapper_types.ModuleType("IPython")
_ipy.get_ipython = get_ipython
_scalene_wrapper_sys.modules["IPython"] = _ipy
for _sub in ("display", "core", "core.display", "core.magic"):
_mod = _scalene_wrapper_types.ModuleType(f"IPython.{_sub}")
# Any attribute access on these modules returns a no-op callable
_mod.__getattr__ = lambda name: (lambda *a, **k: None)
_scalene_wrapper_sys.modules[f"IPython.{_sub}"] = _mod
# Convenience: `display` is the most-imported name; expose it directly.
_scalene_wrapper_sys.modules["IPython.display"].display = lambda *a, **k: None
del _scalene_wrapper_sys, _scalene_wrapper_types
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The current approach to stubbing IPython submodules by assigning __getattr__ to ModuleType instances is ineffective because Python does not use instance-level __getattr__ for module attribute access. This will cause ImportError for statements like from IPython.display import Audio or from IPython.core.magic import register_line_magic.

A more robust approach is to use a custom class that implements __getattr__ and assign its instances to sys.modules. This ensures that any attribute access or import from these mocked modules returns a no-op object that can be called without error.

_IPYTHON_STUB = """\
# Auto-injected by profile_with_scalene.py — stub IPython helpers so
# notebooks converted via nbconvert can run as plain Python under Scalene.
import sys as _scalene_sys

class _NoOp:
    def __getattr__(self, name): return self
    def __call__(self, *args, **kwargs): return None

_stub = _NoOp()
for _name in ["IPython", "IPython.display", "IPython.core", "IPython.core.display", "IPython.core.magic"]:
    if _name not in _scalene_sys.modules:
        _scalene_sys.modules[_name] = _stub

# Ensure get_ipython is available both as a global and on the IPython module
_scalene_sys.modules["IPython"].get_ipython = lambda: _stub
def get_ipython():  # noqa: N802
    return _stub
del _scalene_sys
"""

Notebooks containing `from __future__ import annotations` (or any other
__future__ import) failed to profile because the IPython stub was
prepended to the converted .py, pushing the __future__ imports past line
1 and triggering a SyntaxError.

Fix: walk the converted source for `from __future__` lines and insert
the stub after the last one. When no future imports are present, the
stub still goes at the very top.

Adds a regression test using a synthetic notebook whose first cell line
is `from __future__ import annotations`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@satra

satra commented May 5, 2026

Copy link
Copy Markdown
Collaborator Author

Verified valid — reproduced the SyntaxError with a one-line file import sys ... \nfrom __future__ import annotations. Fixed in ffee86a: the stub is now inserted after any from __future__ import lines via line-walk. Added a regression test (test_wrapper_handles_notebook_with_future_imports) using a synthetic notebook that starts with from __future__ import annotations. Thanks for catching this.


Re. medium-priority comment on module stubbing:

Respectfully disagree — verified empirically that m.__getattr__ = lambda name: ... does work for module attribute access on Python 3.7+ via PEP 562. Test:

import sys, types
m = types.ModuleType("foo")
m.__getattr__ = lambda name: "fallback"
sys.modules["foo"] = m
from foo import bar  # missing attribute
print(bar)  # -> "fallback"

This prints fallback correctly, so from IPython.display import Audio (or any other unfamiliar name) hits the fallback as designed. PEP 562 looks up __getattr__ from the module dict, which is what we set here. Leaving the implementation as-is.

@satra
satra merged commit 1a2955a into alpha May 5, 2026
9 checks passed
@satra
satra deleted the 20260503-235625-scalene-profiling branch May 5, 2026 03:52
github-actions Bot added a commit that referenced this pull request May 5, 2026
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.

1 participant