Skip to content

Commit fbc42e8

Browse files
Improved handling of lines using ipython magics and calls to ipytest (#46)
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
1 parent dd0b26e commit fbc42e8

File tree

9 files changed

+262
-55
lines changed

9 files changed

+262
-55
lines changed

.devcontainer/devcontainer.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
// python
3131
"python.defaultInterpreterPath": "./.venv/bin/python3",
3232
"python.testing.pytestEnabled": true,
33+
"python.testing.autoTestDiscoverOnSaveEnabled": false, // given the work we are doing in pytest-ipynb2 this keeps hanging!
3334
"[python]": {
3435
"editor.rulers": [
3536
120

CHANGELOG.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8-
## [unreleased]
8+
## [0.4.0] - 2025-03-03
99

1010
### Fixed
1111

1212
- Handle notebooks not in test rootdir [#45][pr-45]
13+
- Improved handling of lines using IPython magics and calls to ipytest [#46][pr-46]
14+
15+
[pr-46]: https://github.com/MusicalNinjaDad/pytest-ipynb2/pull/46
1316

1417
[pr-45]: https://github.com/MusicalNinjaDad/pytest-ipynb2/pull/45
1518

@@ -19,7 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1922

2023
[pr-40]: https://github.com/MusicalNinjaDad/pytest-ipynb2/pull/40
2124

22-
## [0.3.0]
25+
## [0.3.0] - 2025-03-01
2326

2427
### Added
2528

README.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,4 @@ For more details see the [docs](https://musicalninjadad.github.io/pytest-ipynb2)
5555
This is an early version. The following things are still on my to-do list:
5656

5757
- Handling tests grouped into classes [#22](https://github.com/MusicalNinjaDad/pytest-ipynb2/issues/22) (might work - I've not checked yet)
58-
- Assertion re-writing. Failed tests don't yet give the expected quality of pytest output about the failure [#20](https://github.com/MusicalNinjaDad/pytest-ipynb2/issues/20) (Workaround - re-run the test using ipytest inside the notebook)
5958
- v1.0.0 will include dedicated commandline options rather than requiring you to specify the plugin [#12](https://github.com/MusicalNinjaDad/pytest-ipynb2/issues/12)
60-
- This won't play nicely with other magics or direct calls to ipytest functions yet [#23](https://github.com/MusicalNinjaDad/pytest-ipynb2/issues/23)

__pyversion__

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
0.3.2
1+
0.4.0

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@
132132
flake8-annotations.mypy-init-return = true
133133
extend-ignore = [
134134
"E701", # One-line ifs are not great, one-liners with suppression are worse
135+
"FLY002", # "\n".join() is much easier to read than a string with loads of "\n" in it.
135136
"ERA001", # pytype suppression blocks are identified as commented-out code
136137
"ANN202", # Return type annotation for private functions
137138
"ANN401", # Using Any often enough that supressing individually is pointless

pytest_ipynb2/_parser.py

Lines changed: 112 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,15 @@
22

33
from __future__ import annotations
44

5+
import ast
6+
from functools import cached_property
57
from typing import TYPE_CHECKING, Protocol, overload
68

9+
import IPython.core.inputtransformer2
710
import nbformat
811

912
if TYPE_CHECKING:
10-
from collections.abc import Generator
13+
from collections.abc import Collection, Generator, Iterator, Sequence
1114
from contextlib import suppress
1215
from pathlib import Path
1316
from typing import SupportsIndex
@@ -16,9 +19,96 @@
1619
from typing import Self
1720

1821

19-
class SourceList(list):
22+
class MagicFinder(ast.NodeVisitor):
23+
"""Identifies lines which use ipython magics or call ipytest."""
24+
25+
def __init__(self) -> None:
26+
self.magiclines: set[int] = set()
27+
"""Linenumbers (starting at 1) of lines containing magics/ipytest."""
28+
self.magicnames = {"get_ipython", "ipytest"}
29+
super().__init__()
30+
31+
def visit_Call(self, node: ast.Call): # noqa: N802
32+
if getattr(node.func, "id", None) in self.magicnames:
33+
self.magiclines.add(node.lineno)
34+
self.generic_visit(node)
35+
36+
def visit_Attribute(self, node: ast.Attribute): # noqa: N802
37+
if getattr(node.value, "id", None) in self.magicnames:
38+
self.magiclines.add(node.lineno)
39+
self.generic_visit(node)
40+
41+
def visit_Import(self, node: ast.Import): # noqa: N802
42+
for mod in node.names:
43+
if mod.name == "ipytest":
44+
self.magiclines.add(node.lineno)
45+
if mod.asname is not None:
46+
self.magicnames.add(mod.asname)
47+
break
48+
self.generic_visit(node)
49+
50+
def visit_ImportFrom(self, node: ast.ImportFrom): # noqa: N802
51+
if node.module in self.magicnames:
52+
self.magiclines.add(node.lineno)
53+
for attr in node.names:
54+
self.magicnames.add(attr.asname if attr.asname is not None else attr.name)
55+
self.generic_visit(node)
56+
57+
58+
class CellSource:
59+
"""
60+
Contains source code of a ipynb cell.
61+
62+
- Initialisable either from a multiline string, or a sequence of strings (one per line)
63+
- String representation is multiline string
64+
- Iterates by line
65+
"""
66+
67+
def __init__(self, contents: Sequence[str] | str):
68+
self._string = contents if isinstance(contents, str) else "\n".join(contents)
69+
70+
def __str__(self) -> str:
71+
return self._string
72+
73+
def __eq__(self, other: object) -> bool:
74+
return str(self) == str(other)
75+
76+
def __hash__(self) -> int:
77+
return hash(self._string) # pragma: no cover
78+
79+
def __iter__(self) -> Iterator[str]:
80+
return iter(self._string.splitlines())
81+
82+
@property
83+
def cellmagiclines(self) -> set[int]:
84+
"""Return a new CellSource with any lines containing cellmagics commented out."""
85+
return {lineno for lineno, line in enumerate(self, start=1) if line.strip().startswith(r"%%")}
86+
87+
@property
88+
def magiclines(self) -> set[int]:
89+
"""Return a list of all lines (starting at 1), the `MagicFinder` identifies."""
90+
transformer = IPython.core.inputtransformer2.TransformerManager()
91+
finder = MagicFinder()
92+
transformed = transformer.transform_cell(str(self))
93+
tree = ast.parse(str(transformed))
94+
finder.visit(tree)
95+
return finder.magiclines
96+
97+
def commentout(self, lines: Collection[int]) -> Self:
98+
return type(self)([f"# {line}" if lineno in lines else line for lineno, line in enumerate(self, start=1)])
99+
100+
@cached_property
101+
def muggled(self) -> Self:
102+
"""A version of this `Source` with magic (and ipytest) lines commented out."""
103+
# Need to handle cell magics first otherwise ipython transformer
104+
# munges the whole cell into a single `run_cell_magic` line
105+
nocellmagics = self.commentout(self.cellmagiclines)
106+
return nocellmagics.commentout(nocellmagics.magiclines)
107+
108+
109+
class SourceList(list[CellSource]):
20110
"""
21-
A `list[str]` with non-continuous indices for storing the contents of cells.
111+
A `list[CellSource]` with non-continuous indices for storing the contents of cells.
22112
23113
- use a full slice `sourcelist[:]`, not list(sourcelist) to get contents.
24114
- supports `.ids()` analog to a mapping.keys(), yielding only cell-ids with source.
@@ -31,10 +121,10 @@ def ids(self) -> Generator[int, None, None]:
31121
yield key
32122

33123
@overload
34-
def __getitem__(self, index: SupportsIndex) -> str: ...
124+
def __getitem__(self, index: SupportsIndex) -> CellSource: ...
35125

36126
@overload
37-
def __getitem__(self, index: slice) -> list[str]: ...
127+
def __getitem__(self, index: slice) -> list[CellSource]: ...
38128

39129
def __getitem__(self, index):
40130
"""
@@ -54,56 +144,46 @@ def __getitem__(self, index):
54144
raise IndexError(msg)
55145
return source
56146

57-
def muggle(self) -> Self:
58-
"""Comment out any ipython magics."""
59-
60-
def _muggleentry(source: str) -> str:
61-
if source is None:
62-
return None
63-
muggled = [
64-
f"# {line}" if (line.strip().startswith("%") or line.strip().startswith("ipytest")) else line
65-
for line in source.splitlines()
66-
]
67-
return "\n".join(muggled)
68-
69-
return type(self)([_muggleentry(source) for source in list(self)])
70-
71147

72148
class Notebook:
73149
"""
74150
The relevant bits of an ipython Notebook.
75151
76152
Attributes:
77-
codecells (SourceList): The code cells *excluding* any identified as test cells.
78-
testcells (SourceList): The code cells which are identified as containing tests, based
79-
upon the presence of the `%%ipytest`magic.
153+
muggled_codecells (SourceList): The code cells *excluding* any identified as test cells.
154+
With magic & ipytest lines commented out.
155+
muggled_testcells (SourceList): The code cells which are identified as containing tests,
156+
based upon the presence of the `%%ipytest` magic. With magic & ipytest lines commented out.
80157
"""
81158

82159
def __init__(self, filepath: Path) -> None:
83-
self.codecells: SourceList
84-
"""The code cells *excluding* any identified as test cells"""
85-
self.testcells: SourceList
86-
"""The code cells which are identified as containing tests, based upon the presence of the `%%ipytest`magic."""
160+
self.muggled_codecells: SourceList
161+
"""The code cells *excluding* any identified as test cells. With magic & ipytest lines commented out."""
162+
self.muggled_testcells: SourceList
163+
"""
164+
The code cells which are identified as containing tests, based upon the presence of the `%%ipytest`magic.
165+
With magic & ipytest lines commented out.
166+
"""
87167

88168
contents = nbformat.read(fp=str(filepath), as_version=4)
89169
nbformat.validate(contents)
90170
cells: list[Cell] = contents.cells
91171

92172
for cell in cells:
93-
cell.source = cell.source.splitlines() # type: ignore[attr-defined] # fulfils protocol after splitlines
173+
cell.source = CellSource(cell.source) # type: ignore[attr-defined] # fulfils protocol after this conversion
94174

95175
def _istestcell(cell: Cell) -> bool:
96176
return cell.cell_type == "code" and any(line.strip().startswith(r"%%ipytest") for line in cell.source)
97177

98178
def _iscodecell(cell: Cell) -> bool:
99179
return cell.cell_type == "code"
100180

101-
self.codecells = SourceList(
102-
"\n".join(cell.source) if _iscodecell(cell) and not _istestcell(cell) else None for cell in cells
103-
).muggle()
104-
self.testcells = SourceList("\n".join(cell.source) if _istestcell(cell) else None for cell in cells).muggle()
181+
self.muggled_codecells = SourceList(
182+
cell.source.muggled if _iscodecell(cell) and not _istestcell(cell) else None for cell in cells
183+
)
184+
self.muggled_testcells = SourceList(cell.source.muggled if _istestcell(cell) else None for cell in cells)
105185

106186

107187
class Cell(Protocol):
108-
source: list[str]
188+
source: CellSource
109189
cell_type: str

pytest_ipynb2/plugin.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ def _linecache_getlines_ipynb2(filename: str, module_globals: dict | None = None
120120
if CellPath.is_cellpath(filename):
121121
notebook = CellPath.get_notebookpath(filename)
122122
cellid = CellPath.get_cellid(filename)
123-
return _ParsedNotebook(notebook).testcells[cellid].splitlines()
123+
return list(_ParsedNotebook(notebook).muggled_testcells[cellid])
124124
return _linecache_getlines_std(filename=filename, module_globals=module_globals)
125125

126126
linecache.getlines = _linecache_getlines_ipynb2
@@ -201,7 +201,7 @@ class Notebook(pytest.File):
201201
def collect(self) -> Generator[Cell, None, None]:
202202
"""Yield `Cell`s for all cells which contain tests."""
203203
parsed = _ParsedNotebook(self.path)
204-
for testcellid in parsed.testcells.ids():
204+
for testcellid in parsed.muggled_testcells.ids():
205205
name = f"{CELL_PREFIX}{testcellid}"
206206
nodeid = f"{self.nodeid}::{name}"
207207
cell = Cell.from_parent(
@@ -240,8 +240,8 @@ def _getobj(self) -> ModuleType:
240240
notebook = self.stash[ipynb2_notebook]
241241
cellid = self.stash[ipynb2_cellid]
242242

243-
cellsabove = notebook.codecells[:cellid]
244-
testcell_source = notebook.testcells[cellid]
243+
cellsabove = [str(cellsource) for cellsource in notebook.muggled_codecells[:cellid]]
244+
testcell_source = str(notebook.muggled_testcells[cellid])
245245

246246
cell_filename = str(self.path)
247247

tests/test_fixtures.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
{
3838
"generated.ipynb": [
3939
add_ipytest_magic(
40-
"\n".join( # noqa: FLY002
40+
"\n".join(
4141
[
4242
"def test_pass():",
4343
" assert True",
@@ -59,15 +59,15 @@
5959
),
6060
{
6161
"generated.ipynb": [
62-
"\n".join( # noqa: FLY002
62+
"\n".join(
6363
[
6464
"import ipytest",
6565
"ipytest.autoconfig()",
6666
"",
6767
],
6868
),
6969
add_ipytest_magic(
70-
"\n".join( # noqa: FLY002
70+
"\n".join(
7171
[
7272
"def test_pass():",
7373
" assert True",

0 commit comments

Comments
 (0)