Skip to content

Commit 4b608a1

Browse files
martin-s-aclaude
andauthored
Open the em.image module with the types read and write need (#148)
First half of #143. Opens `em` and binds the types `read` and `write` take, without binding either function — that is the second half. Everything here is independently reachable and tested, so the API decisions land before the functions that depend on them. Follows the three answers on the issue: the format managers are services like any other, `image_metadata` stays out, and nothing parses a location out of a string on the Python side. ## What is bound | Name | Is | |---|---| | `rexlib.em.image.ImageLocation` | Path plus a zero based index, `no_position` for the whole file | | `rexlib.em.image.parse_image_location` | `str` to `ImageLocation | None` | | `rexlib.em.image.ImageReadFormatManager` | Opaque handle, like `ProgramManager` | | `rexlib.em.image.ImageWriteFormatManager` | Same, for writing | | `rexlib.em.image.get_image_{read,write}_format_manager` | `ServiceCatalog` to the manager | The managers are bound with no methods at all. `open` would drag in `image_reader`, and `get_most_suitable_format` `image_probe` and `image_read_format`; none of that is needed to pass a manager to `read`, which is all the issue asks for. `ProgramManager` is bound exactly this way today. ## The module path `em/image/`, not `em/`, which is the decision argued on the issue and not yet confirmed there — flag it if you disagree, it is cheap now and expensive once released. Short version: the submodules follow rexlib's directories, and the area has to be the module path rather than part of the function name, or `read` becomes `read_image` the day metadata reading lands. This is the first public subpackage in `python/rexlib/`, where the convention so far is a flat set of private modules re-exported by `__init__.py`. Here the path is the API, so `em/` and `em/image/` are named without an underscore and carry `__all__` of their own. `AGENTS.md` records the exception. ## No parsing, deliberately `DeviceIndex` sets the opposite precedent: it takes a second `py::init` that parses `"cuda:0"` and throws on nonsense. `ImageLocation` does not, because `ImageLocation("3@stack.mrc")` would be Python inventing a conversion that C++ does not have, and because the primary constructor already gives a single string the meaning "this path, whole file" — the two would be ambiguous rather than complementary. `parse_image_location` is asked for by name and returns `None` rather than throwing, since failing to parse is a routine outcome here where `DeviceIndex`'s is an error. A test pins it: ```python def test_a_path_is_never_parsed_on_its_way_into_a_location(): assert ImageLocation('3@stack.mrc').path == '3@stack.mrc' ``` ## The stub collection had to change This is the first submodule holding a submodule, and pybind11-stubgen renders that as a directory rather than a file. Verified against 2.5.5, the pinned version: ``` _binding/__init__.pyi _binding/numerical.pyi _binding/em/__init__.pyi <- new shape _binding/em/image.pyi ``` `deploy.yml` collected stubs with `cp .../_binding/*.pyi stubs/`, a flat glob that would have taken the top level and silently dropped `em`. Nothing would have failed: `install_python_stubs` only checks that *some* `.pyi` exists, and `install(DIRECTORY ... FILES_MATCHING)` is recursive, so every published wheel would have shipped typed except for the area this PR adds. It now copies the tree. ## Checked - `ruff check .` passes. - Every new and modified translation unit compiles clean under `g++ -std=c++20 -fsyntax-only -Wall -Wextra -Wpedantic` against rexlib's headers and the pinned pybind11. - The parse test cases are taken from `parse_image_location`'s actual rejections — empty index, index zero, non-digits — not guessed. - 28 tests collect under `tests/em/image/`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 80c8953 commit 4b608a1

20 files changed

Lines changed: 586 additions & 17 deletions

‎.github/workflows/deploy.yml‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,16 +90,13 @@ jobs:
9090
key: stubs-ubuntu-latest
9191

9292
- name: Collect the generated stubs
93-
run: |
94-
mkdir -p stubs
95-
cp "$(python -c 'import rexlib, pathlib; print(pathlib.Path(rexlib.__file__).parent)')"/_binding/*.pyi stubs/
96-
ls -l stubs
93+
run: python scripts/collect_stubs.py --module rexlib._binding --output stubs
9794

9895
- name: Upload stubs
9996
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
10097
with:
10198
name: type-stubs
102-
path: stubs/*.pyi
99+
path: stubs/
103100

104101
build_wheels:
105102
needs: generate_stubs

‎AGENTS.md‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ the same pull request that causes it.
1414

1515
| Path | Holds |
1616
|---|---|
17-
| `src/` | The pybind11 binding, 35 `.cpp` and 35 `.hpp`, compiled into the `rexlib._binding` extension |
18-
| `python/rexlib/` | The Python package, 8 `.py`: everything the binding cannot express |
17+
| `src/` | The pybind11 binding, 40 `.cpp` and 40 `.hpp`, compiled into the `rexlib._binding` extension |
18+
| `python/rexlib/` | The Python package, 10 `.py`: everything the binding cannot express |
1919
| `tests/` | pytest suites, mirroring the binding's module structure |
2020
| `tests/assets/` | Two dummy plugins, built by CMake, that the plugin tests discover |
21-
| `scripts/` | Development tools: the test runner, the stub generator, the wheel check |
21+
| `scripts/` | Development tools: the test runner, the stub generator and collector, the wheel check |
2222
| `cmake/modules/` | `python_stubs.cmake`, which installs the type stubs |
2323
| `conf/` | `coverage.ini` |
2424
| `external/rexlib` | The rexlib the repository is developed against, a submodule |
@@ -29,9 +29,15 @@ to ask.
2929
`src/` mirrors rexlib's own directory structure, and the module it produces
3030
mirrors the C++ API: same names, same argument order, same required arguments.
3131
It does not decide anything. `python/rexlib/` is where the API becomes Python:
32-
default arguments, implicit context, operators, process-wide state. Everything
33-
in it is private — a leading underscore on every module — and `__init__.py` is
34-
the whole public surface.
32+
default arguments, implicit context, operators, process-wide state. Its
33+
modules carry a leading underscore and `__init__.py` re-exports what they
34+
hold, so that the public surface is one flat namespace rather than a set of
35+
import paths.
36+
37+
A public subpackage is the exception to that, and `em/` is the one: its module
38+
path *is* the API, `rexlib.em.image`, so the directories are named without an
39+
underscore and each carries an `__all__` of its own. Anything private inside
40+
one still takes the underscore.
3541

3642
The rule that follows: if a change could be expressed in either place, it goes
3743
in `python/rexlib/`. C++ is the expensive side to change and the one that has
@@ -47,10 +53,12 @@ them: everything under those directories is flat in `namespace rexlib`,
4753
the only place rexlib separates these areas, so the paths are what the
4854
submodule names come from.
4955

50-
Nothing under `em/` is bound yet. When it is, the same rule decides its module
51-
path: `em/image/` sits inside `namespace em` the way `core/hardware/` sits
52-
inside `namespace rexlib`, and the path is what will separate it from the
53-
areas that come after it. See #143.
56+
It reaches through nesting where rexlib's directories nest. `em/image/` sits
57+
inside `namespace em` the way `core/hardware/` sits inside `namespace rexlib`,
58+
so it binds as `em.image` rather than as `em`, even though `em::read` carries
59+
no `image` qualifier of its own. The path is what keeps images apart from the
60+
areas that come after them, which is why it is the path and not the function
61+
name that names them. See #143.
5462

5563
`_binding` carries at its top level what the directories below it sit in:
5664

@@ -62,9 +70,12 @@ areas that come after it. See #143.
6270
| `_binding.hardware` | `src/core/hardware/` | Devices, sessions, queues, events, memory resources |
6371
| `_binding.dispatch` | `src/core/dispatch/` | `ExecutionContext`, `Dispatcher`, `ProgramManager` |
6472
| `_binding.functional` | `src/functional/` | The operations, each taking an explicit context |
73+
| `_binding.em.image` | `src/em/image/` | `ImageLocation`, the read and write format managers |
6574

66-
`src/main.cpp` is the only place that creates a submodule, and it names them
67-
in the order the declarations need.
75+
A submodule is created by the `main.cpp` above it, which then hands it to the
76+
`bind_` function of the directory it stands for: `src/main.cpp` creates `em`
77+
and `src/em/main.cpp` creates `image` inside it. Each names its submodules in
78+
the order the declarations need.
6879

6980
### The package's modules
7081

@@ -77,6 +88,7 @@ in the order the declarations need.
7788
| `_device.py` | `rexlib.device(...)`, the `with` block that activates one |
7889
| `_functional.py` | The operations again, with `context` defaulting to the active one |
7990
| `_ndarray.py` | Installs the Python operators onto `Array` |
91+
| `em/` | The electron microscopy areas, one module each |
8092

8193
`_paths` is imported first in `__init__.py`, and the order matters: on Windows
8294
nothing else imports until the bundled library is findable. `_ndarray` is
@@ -160,6 +172,14 @@ The package is typed — `py.typed` ships — so a change to the binding's
160172
signatures is a change to the stubs, generated for free, and a change to
161173
`python/rexlib/` is only as typed as it was written.
162174

175+
A submodule that holds submodules of its own becomes a directory of stubs
176+
rather than one file: `em` is `_binding/em/__init__.pyi` beside
177+
`_binding/em/image.pyi`. Every step that moves stubs has to carry that tree,
178+
so `scripts/collect_stubs.py` copies it and the artifact holding it is
179+
uploaded as a directory. A flat `*.pyi` glob at either step takes the top
180+
level and drops the rest, which fails nothing and publishes wheels typed
181+
everywhere except the area that was just added.
182+
163183
## Conventions
164184

165185
### The binding
@@ -281,6 +301,7 @@ Tests for what `_binding` exposes at its top level stay at the root.
281301
| `tests/hardware/` | Devices, sessions, events, memory resources, the session pool |
282302
| `tests/dispatch/` | `ExecutionContext`, the active context, `rexlib.device(...)` |
283303
| `tests/functional/` | The operations |
304+
| `tests/em/image/` | `ImageLocation`, parsing, the format managers |
284305
285306
There are no `__init__.py` files and no `conftest.py`. Test module names are
286307
therefore unique across the whole tree, and a fixture belongs to the file that

‎python/rexlib/__init__.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
itself, and the shape of the array libraries they are used alongside.
1313
Device management and dispatch stay in their own modules: they are
1414
reached for when setting up rather than when computing.
15+
16+
`em` is a package rather than a flat name, one module per area of
17+
electron microscopy, so that each area keeps its own `read` and `write`
18+
instead of spelling the area into the function name.
1519
"""
1620

1721
from __future__ import annotations
@@ -39,6 +43,8 @@
3943
from ._device import device as device
4044
from ._paths import get_cmake_dir as get_cmake_dir, get_include as get_include
4145

46+
from . import em as em
47+
4248
# Imported for its effect: it installs the Python operators onto Array,
4349
# which has to happen before anything hands one out.
4450
from . import _ndarray as _ndarray
@@ -89,6 +95,7 @@
8995
"device",
9096
"dispatch",
9197
"divide",
98+
"em",
9299
"empty",
93100
"fill",
94101
"full",

‎python/rexlib/em/__init__.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# SPDX-License-Identifier: GPL-3.0-only
2+
3+
"""Electron microscopy file formats.
4+
5+
One module per area of `rexlib::em`, named after the directory that area
6+
occupies in rexlib, the way every module here is named. Each carries the
7+
vocabulary of its own area, so `read` and `write` mean one thing apiece
8+
and the areas arriving after images rename nothing.
9+
"""
10+
11+
from __future__ import annotations
12+
13+
from . import image as image
14+
15+
__all__ = ["image"]

‎python/rexlib/em/image/__init__.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# SPDX-License-Identifier: GPL-3.0-only
2+
3+
"""Image files: where one is, and which formats are known.
4+
5+
`ImageLocation` addresses an element of a file, and the two format
6+
managers hold the formats that can be read and written. Both managers are
7+
services, reached from a `rexlib.ServiceCatalog` the way the device and
8+
program managers are.
9+
10+
A string is never parsed into an `ImageLocation` on its way anywhere: a
11+
path is a path, and `parse_image_location` is asked for by name.
12+
"""
13+
14+
from __future__ import annotations
15+
16+
from ..._binding.em.image import (
17+
ImageLocation as ImageLocation,
18+
ImageReadFormatManager as ImageReadFormatManager,
19+
ImageWriteFormatManager as ImageWriteFormatManager,
20+
get_image_read_format_manager as get_image_read_format_manager,
21+
get_image_write_format_manager as get_image_write_format_manager,
22+
parse_image_location as parse_image_location,
23+
)
24+
25+
__all__ = [
26+
"ImageLocation",
27+
"ImageReadFormatManager",
28+
"ImageWriteFormatManager",
29+
"get_image_read_format_manager",
30+
"get_image_write_format_manager",
31+
"parse_image_location",
32+
]

‎scripts/collect_stubs.py‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
#!/usr/bin/env python3
2+
# SPDX-License-Identifier: GPL-3.0-only
3+
4+
"""
5+
Copy an extension's type stubs out of an installed package.
6+
7+
pybind11-stubgen renders a submodule holding submodules of its own as a
8+
directory of stubs rather than as one file, so what it writes is a tree and
9+
this carries the tree. Invoked from the workflow that generates the stubs
10+
once and hands them to the builds that cannot generate their own.
11+
"""
12+
13+
from __future__ import annotations
14+
15+
import argparse
16+
import importlib
17+
import pathlib
18+
import shutil
19+
20+
def parse_arguments() -> argparse.Namespace:
21+
"""Read the module whose stubs to copy, and where to put them."""
22+
parser = argparse.ArgumentParser(description=__doc__)
23+
parser.add_argument("--module", required=True)
24+
parser.add_argument(
25+
"--output",
26+
required=True,
27+
help="Directory the stubs are copied to.",
28+
)
29+
return parser.parse_args()
30+
31+
def find_stubs(module: str) -> pathlib.Path:
32+
"""Locate the stub directory installed beside an extension."""
33+
package, _, name = module.rpartition(".")
34+
root = importlib.import_module(package)
35+
if root.__file__ is None:
36+
raise SystemExit(f"{package} has no location on disk")
37+
return pathlib.Path(root.__file__).parent / name
38+
39+
def copy_stubs(source: pathlib.Path, destination: pathlib.Path) -> int:
40+
"""Copy every stub under source, keeping the path it sits at."""
41+
copied = 0
42+
for stub in sorted(source.rglob("*.pyi")):
43+
target = destination / stub.relative_to(source)
44+
target.parent.mkdir(parents=True, exist_ok=True)
45+
shutil.copyfile(stub, target)
46+
print(target)
47+
copied += 1
48+
return copied
49+
50+
def main() -> None:
51+
"""Copy the stubs, failing when there is not one to copy."""
52+
arguments = parse_arguments()
53+
54+
source = find_stubs(arguments.module)
55+
if not source.is_dir():
56+
raise SystemExit(f"No stub directory at {source}")
57+
58+
if not copy_stubs(source, pathlib.Path(arguments.output)):
59+
raise SystemExit(f"No .pyi files under {source}")
60+
61+
if __name__ == "__main__":
62+
main()

‎src/em/image/image_location.cpp‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
// SPDX-License-Identifier: GPL-3.0-only
2+
3+
#include "image_location.hpp"
4+
5+
#include <rexlib/em/image/image_location.hpp>
6+
7+
#include <pybind11/operators.h>
8+
#include <pybind11/stl.h>
9+
10+
#include <optional>
11+
#include <sstream>
12+
#include <string>
13+
14+
namespace rexlib
15+
{
16+
17+
namespace py = pybind11;
18+
19+
static std::string to_string(const em::image_location &l)
20+
{
21+
return em::to_string(l);
22+
}
23+
24+
static std::string to_repr(const em::image_location &l)
25+
{
26+
std::ostringstream oss;
27+
oss << "ImageLocation(path=\"" << l.get_path() << "\", "
28+
<< "position=" << l.get_position_in_stack() << ")";
29+
return oss.str();
30+
}
31+
32+
static std::optional<em::image_location>
33+
parse_image_location(const std::string &text)
34+
{
35+
em::image_location result;
36+
if (!em::parse_image_location(text, result))
37+
{
38+
return std::nullopt;
39+
}
40+
return result;
41+
}
42+
43+
image_location_class declare_image_location(pybind11::module_ &m)
44+
{
45+
return image_location_class(m, "ImageLocation");
46+
}
47+
48+
void define_image_location(image_location_class &c, pybind11::module_ &m)
49+
{
50+
c
51+
.def(
52+
py::init<std::string, std::size_t>(),
53+
py::arg("path"),
54+
py::arg("position") = em::image_location::no_position
55+
)
56+
.def(py::init<>())
57+
.def(py::self == py::self)
58+
.def(py::self != py::self)
59+
.def(py::self < py::self)
60+
.def(py::self <= py::self)
61+
.def(py::self > py::self)
62+
.def(py::self >= py::self)
63+
.def("__hash__", &em::image_location::hash)
64+
.def("__str__", &to_string)
65+
.def("__repr__", &to_repr)
66+
.def_property_readonly("path", &em::image_location::get_path)
67+
.def_property_readonly(
68+
"position_in_stack",
69+
&em::image_location::get_position_in_stack
70+
)
71+
.def(py::pickle(
72+
[](const em::image_location &l) // __getstate__
73+
{
74+
return py::make_tuple(
75+
l.get_path(),
76+
l.get_position_in_stack()
77+
);
78+
},
79+
[](py::tuple t) // __setstate__
80+
{
81+
return em::image_location(
82+
t[0].cast<std::string>(),
83+
t[1].cast<std::size_t>()
84+
);
85+
}
86+
));
87+
88+
c.attr("no_position") = em::image_location::no_position;
89+
90+
m.def("parse_image_location", &parse_image_location, py::arg("text"));
91+
}
92+
93+
} // namespace rexlib

‎src/em/image/image_location.hpp‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// SPDX-License-Identifier: GPL-3.0-only
2+
3+
#pragma once
4+
5+
#include <pybind11/pybind11.h>
6+
7+
#include <rexlib/em/image/image_location.hpp>
8+
9+
namespace rexlib
10+
{
11+
12+
using image_location_class = pybind11::class_<em::image_location>;
13+
14+
image_location_class declare_image_location(pybind11::module_ &m);
15+
void define_image_location(image_location_class &c, pybind11::module_ &m);
16+
17+
} // namespace rexlib
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// SPDX-License-Identifier: GPL-3.0-only
2+
3+
#include "image_read_format_manager.hpp"
4+
5+
#include <rexlib/core/service_catalog.hpp>
6+
7+
namespace rexlib
8+
{
9+
10+
static std::shared_ptr<em::image_read_format_manager>
11+
get_image_read_format_manager(service_catalog &catalog)
12+
{
13+
return catalog.get_service_manager<em::image_read_format_manager>();
14+
}
15+
16+
image_read_format_manager_class
17+
declare_image_read_format_manager(pybind11::module_ &m)
18+
{
19+
return image_read_format_manager_class(m, "ImageReadFormatManager");
20+
}
21+
22+
void define_image_read_format_manager(pybind11::module_ &m)
23+
{
24+
m.def("get_image_read_format_manager", &get_image_read_format_manager);
25+
}
26+
27+
} // namespace rexlib

0 commit comments

Comments
 (0)