diff --git a/cppwg/genpackage.py b/cppwg/genpackage.py index 008b23f..b1c8488 100644 --- a/cppwg/genpackage.py +++ b/cppwg/genpackage.py @@ -19,7 +19,12 @@ * shared-module split: one compiled extension (e.g. ``_pychaste_all``) is split into several subpackages by a layout that lists which names each owns; imported explicitly with ``from . import (...)``. Used by - pychaste. + pychaste. Such a layout may also carry an ``exclude`` list of wrapped names + that are deliberately not exposed (e.g. abstract base classes): they are still + registered in the compiled extension - concrete subclasses depend on that - but + are kept out of the Python package namespace. genpackage does not warn that an + excluded name is unplaced, but does warn if one is nonetheless assigned to a + subpackage (i.e. exposed after all). Usage:: @@ -225,6 +230,14 @@ def generate_module_per_subpackage(model: dict, layout: dict, overwrite: bool) - diagonal_shorthand = layout.get("diagonal_shorthand", False) cpp_to_pyname = _build_cpp_to_pyname(model) + # `exclude` cannot be honored here: a module-per-subpackage layout exposes + # each module wholesale via `from . import *`, so individual names + # cannot be held back at the Python layer. It is only meaningful for the + # shared-module split (explicit per-name membership). + if layout.get("exclude"): + print("warning: 'exclude' is only honored for shared-module-split layouts; " + "ignoring it for this module-per-subpackage layout", file=sys.stderr) + written = [] for module in model["modules"]: # The subpackage directory (relative to package_root); default = module @@ -308,13 +321,33 @@ def generate_shared_module_split(model: dict, layout: dict, overwrite: bool) -> written.append(_write_generated(path, content, overwrite)) flatten_names[subpkg] = exported - # Flag any wrapped class not placed in a subpackage, so nothing is silently - # dropped when the config gains a class. - unplaced = sorted(set(classes_by_base) | other_names) - for name in unplaced: - if name not in assigned: - print(f"warning: '{name}' is wrapped but not assigned to a subpackage", - file=sys.stderr) + # Classes intentionally left unexposed (e.g. abstract base classes, which + # should not be instantiated or subclassed from Python). Listing them in the + # layout's `exclude` makes their absence from every subpackage deliberate + # rather than an oversight, and lets exposing one later be flagged (below). + excluded = set(layout.get("exclude", [])) + known = set(classes_by_base) | other_names + + # Flag any wrapped class neither placed in a subpackage nor explicitly + # excluded, so nothing is silently dropped when the config gains a class. + for name in sorted(known - assigned - excluded): + print(f"warning: '{name}' is wrapped but not assigned to a subpackage", + file=sys.stderr) + + # Flag an excluded name that is nonetheless assigned to a subpackage: it is + # exposed despite being marked unexposed. Intersect with `known`: a name + # assigned but absent from the model was never actually imported (it already + # draws "not found in model" and the stale-entry warning below), so it is not + # exposed and must not be reported here. + for name in sorted(excluded & assigned & known): + print(f"warning: '{name}' is in the layout 'exclude' list but is also " + f"assigned to a subpackage, so it is exposed", file=sys.stderr) + + # Flag a stale exclude entry (not a wrapped entity) - likely a class that was + # renamed or removed - so the exclude list is kept in step with the wrappers. + for name in sorted(excluded - known): + print(f"warning: '{name}' is in the layout 'exclude' list but is not a " + f"wrapped class, enum or free function in the model", file=sys.stderr) if layout.get("flatten_to_root"): written.append( diff --git a/doc/python-packages.md b/doc/python-packages.md index 16b6f51..919e884 100644 --- a/doc/python-packages.md +++ b/doc/python-packages.md @@ -212,6 +212,33 @@ class Element(TemplateClass): As in the per-module layout, a hand-written `__init__.py` in each subpackage star-imports this via `from ._generated import *`. +### `exclude` + +Some wrapped classes are deliberately **not** exposed in the Python package. For +example, abstract base classes must typically stay wrapped in the compiled +extension because concrete C++ subclasses declare them as bases, and C++ APIs +pass and return them. However, in most cases they are not meant to be named, +instantiated or subclassed from Python. List such classes under `exclude` so +their absence from every subpackage is recognised as intentional rather than an +oversight: + +```yaml +compiled_module: _pychaste_all +exclude: + - AbstractBoundaryCondition + - AbstractBoxDomainPdeModifier + # ... +subpackages: + # ... concrete classes only +``` + +An excluded name is not warned about for being unplaced. genpackage still warns +if an excluded name is **also** assigned to a subpackage (it would be exposed +after all) or if an `exclude` entry no longer matches any wrapped class (e.g. +after a rename). `exclude` applies only to this shared-extension split; a +module-per-subpackage layout exposes each module wholesale via `import *`, so it +cannot hold individual names back. + (hand-written-code)= ## Hand-written code diff --git a/examples/cells/src/py/pycells/_syntax.py b/examples/cells/src/py/pycells/_syntax.py index 8173e9d..c51d09f 100644 --- a/examples/cells/src/py/pycells/_syntax.py +++ b/examples/cells/src/py/pycells/_syntax.py @@ -9,10 +9,10 @@ normalization that resolves a subscript to the concrete instantiation. """ -from collections.abc import Iterable +from collections.abc import Callable, Iterable -def _normalize_key(key): +def _normalize_key(key: object) -> tuple[str, ...]: """Normalize a template-argument subscript key to a tuple of strings. A scalar key becomes a 1-tuple; each argument maps to its ``__name__`` (for a @@ -41,13 +41,13 @@ class TemplateClass: _instantiations: dict = {} - def __init_subclass__(cls, **kwargs): + def __init_subclass__(cls, **kwargs) -> None: super().__init_subclass__(**kwargs) cls._instantiations = { _normalize_key(args): cls_ for args, cls_ in cls._instantiations.items() } - def __class_getitem__(cls, key): + def __class_getitem__(cls, key: object) -> type: return cls._instantiations[_normalize_key(key)] @@ -69,11 +69,15 @@ class TemplateMethod: >>> foo_obj.Bar(arg) # the plain overload, via the fallback """ - def __init__(self, base_name, fallback=None): + def __init__( + self, base_name: str, fallback: Callable[..., object] | None = None + ) -> None: self._base_name = base_name # e.g. "Bar" for foo_obj.Bar[T]() self._fallback = fallback # a plain overload of the same name, or None - def __get__(self, obj, owner=None): + def __get__( + self, obj: object | None, owner: type | None = None + ) -> "_BoundTemplateMethod": # Bar is a descriptor on the class, so accessing ``foo_obj.Bar`` triggers # __get__, returning a _BoundTemplateMethod. obj is the instance, or None # when accessed on the class itself (``Foo.Bar``); owner is the class. @@ -81,21 +85,28 @@ def __get__(self, obj, owner=None): class _BoundTemplateMethod: - def __init__(self, obj, owner, base_name, fallback): + def __init__( + self, + obj: object | None, + owner: type, + base_name: str, + fallback: Callable[..., object] | None, + ) -> None: self._obj = obj # the instance, or None when accessed on the class - # The mangled bindings live on the instance's class; look them up on the - # instance (instance access) or the class itself (class access). - self._target = obj if obj is not None else owner + self._owner = owner # the class self._base_name = base_name # e.g. "Bar" for foo_obj.Bar[T]() self._fallback = fallback - def __getitem__(self, key): + def __getitem__(self, key: object) -> Callable[..., object]: # The [T] subscript on ``foo_obj.Bar[T]()`` triggers __getitem__, # returning the target.Bar_T method, the binding generated by cppwg. suffix = "_" + "_".join(_normalize_key(key)) # e.g. _T - return getattr(self._target, self._base_name + suffix) + # The mangled bindings live on the instance's class; look them up on the + # instance (instance access) or the class itself (class access). + target = self._obj if self._obj is not None else self._owner + return getattr(target, self._base_name + suffix) - def __call__(self, *args, **kwargs): + def __call__(self, *args: object, **kwargs: object) -> object: # ``foo_obj.Bar(...)`` with no subscript calls the plain overload kept as # the fallback; with no fallback the name is purely templated, so point # the caller at the subscript form. diff --git a/examples/shapes/src/py/pyshapes/_syntax.py b/examples/shapes/src/py/pyshapes/_syntax.py index 8173e9d..c51d09f 100644 --- a/examples/shapes/src/py/pyshapes/_syntax.py +++ b/examples/shapes/src/py/pyshapes/_syntax.py @@ -9,10 +9,10 @@ normalization that resolves a subscript to the concrete instantiation. """ -from collections.abc import Iterable +from collections.abc import Callable, Iterable -def _normalize_key(key): +def _normalize_key(key: object) -> tuple[str, ...]: """Normalize a template-argument subscript key to a tuple of strings. A scalar key becomes a 1-tuple; each argument maps to its ``__name__`` (for a @@ -41,13 +41,13 @@ class TemplateClass: _instantiations: dict = {} - def __init_subclass__(cls, **kwargs): + def __init_subclass__(cls, **kwargs) -> None: super().__init_subclass__(**kwargs) cls._instantiations = { _normalize_key(args): cls_ for args, cls_ in cls._instantiations.items() } - def __class_getitem__(cls, key): + def __class_getitem__(cls, key: object) -> type: return cls._instantiations[_normalize_key(key)] @@ -69,11 +69,15 @@ class TemplateMethod: >>> foo_obj.Bar(arg) # the plain overload, via the fallback """ - def __init__(self, base_name, fallback=None): + def __init__( + self, base_name: str, fallback: Callable[..., object] | None = None + ) -> None: self._base_name = base_name # e.g. "Bar" for foo_obj.Bar[T]() self._fallback = fallback # a plain overload of the same name, or None - def __get__(self, obj, owner=None): + def __get__( + self, obj: object | None, owner: type | None = None + ) -> "_BoundTemplateMethod": # Bar is a descriptor on the class, so accessing ``foo_obj.Bar`` triggers # __get__, returning a _BoundTemplateMethod. obj is the instance, or None # when accessed on the class itself (``Foo.Bar``); owner is the class. @@ -81,21 +85,28 @@ def __get__(self, obj, owner=None): class _BoundTemplateMethod: - def __init__(self, obj, owner, base_name, fallback): + def __init__( + self, + obj: object | None, + owner: type, + base_name: str, + fallback: Callable[..., object] | None, + ) -> None: self._obj = obj # the instance, or None when accessed on the class - # The mangled bindings live on the instance's class; look them up on the - # instance (instance access) or the class itself (class access). - self._target = obj if obj is not None else owner + self._owner = owner # the class self._base_name = base_name # e.g. "Bar" for foo_obj.Bar[T]() self._fallback = fallback - def __getitem__(self, key): + def __getitem__(self, key: object) -> Callable[..., object]: # The [T] subscript on ``foo_obj.Bar[T]()`` triggers __getitem__, # returning the target.Bar_T method, the binding generated by cppwg. suffix = "_" + "_".join(_normalize_key(key)) # e.g. _T - return getattr(self._target, self._base_name + suffix) + # The mangled bindings live on the instance's class; look them up on the + # instance (instance access) or the class itself (class access). + target = self._obj if self._obj is not None else self._owner + return getattr(target, self._base_name + suffix) - def __call__(self, *args, **kwargs): + def __call__(self, *args: object, **kwargs: object) -> object: # ``foo_obj.Bar(...)`` with no subscript calls the plain overload kept as # the fallback; with no fallback the name is purely templated, so point # the caller at the subscript form. diff --git a/tests/test_genpackage.py b/tests/test_genpackage.py index 8843cc0..bfd37ca 100644 --- a/tests/test_genpackage.py +++ b/tests/test_genpackage.py @@ -388,6 +388,105 @@ def test_shared_split_warns_on_unknown_and_unassigned(tmp_path, capsys): assert "'Orphan'" in err # in model but not assigned to a subpackage +def _split_model_with_abstract(tmp_path): + """A shared-split model + layout: one exposed class, one excluded abstract base.""" + model = { + "package": "pkg", + "modules": [ + { + "name": "all", + "compiled_module": "_pkg_all", + "imports": [], + "classes": [ + _class("Kept", [_inst([], "Kept")], templated=False), + _class("AbstractBase", [_inst([], "AbstractBase")], templated=False), + ], + "enums": [], + "free_functions": [], + } + ], + } + layout = { + "package": "pkg", + "package_root": str(tmp_path), + "compiled_module": "_pkg_all", + "subpackages": {"sub": ["Kept"]}, + "exclude": ["AbstractBase"], + } + return model, layout + + +def test_shared_split_excluded_class_not_warned_as_unassigned(tmp_path, capsys): + """An excluded (deliberately unexposed) class is not flagged as an orphan.""" + model, layout = _split_model_with_abstract(tmp_path) + genpackage.generate_shared_module_split(model, layout, overwrite=False) + + err = capsys.readouterr().err + assert "AbstractBase" not in err # excluded -> its absence is deliberate + assert "not assigned" not in err + + +def test_shared_split_warns_when_excluded_class_is_also_exposed(tmp_path, capsys): + """Assigning an excluded class to a subpackage exposes it -> warn (the guard).""" + model, layout = _split_model_with_abstract(tmp_path) + layout["subpackages"]["sub"].append("AbstractBase") # now exposed despite exclude + genpackage.generate_shared_module_split(model, layout, overwrite=False) + + err = capsys.readouterr().err + assert "'AbstractBase'" in err + assert "exclude" in err and "exposed" in err + + +def test_shared_split_warns_on_stale_exclude_entry(tmp_path, capsys): + """An exclude entry that no longer names a wrapped class is flagged as stale.""" + model, layout = _split_model_with_abstract(tmp_path) + layout["exclude"].append("RenamedAway") # not in the model + genpackage.generate_shared_module_split(model, layout, overwrite=False) + + err = capsys.readouterr().err + assert "'RenamedAway'" in err + assert "not a" in err # "... not a wrapped class, enum or free function" + + +def test_shared_split_stale_name_in_both_lists_not_reported_as_exposed(tmp_path, capsys): + """A stale name in both `subpackages` and `exclude` was never imported, so it + is flagged as not-found and stale, but never as "exposed".""" + model, layout = _split_model_with_abstract(tmp_path) + layout["subpackages"]["sub"].append("RenamedAway") # assigned but not in model + layout["exclude"].append("RenamedAway") # ... and also excluded + genpackage.generate_shared_module_split(model, layout, overwrite=False) + + err = capsys.readouterr().err + assert "not found in model" in err # accurate: it was omitted from the imports + assert "not a wrapped class" in err # accurate: stale exclude entry + assert "exposed" not in err # it is NOT exposed, so must not claim it is + + +def test_module_per_subpackage_warns_that_exclude_is_ignored(tmp_path, capsys): + """`exclude` is meaningless for a wildcard module-per-subpackage layout.""" + model = { + "package": "pkg", + "modules": [ + { + "name": "geometry", + "compiled_module": "_pkg_geometry", + "imports": [], + "classes": [_class("Point", [_inst([], "Point")], templated=False)], + "enums": [], + "free_functions": [], + } + ], + } + layout = { + "package": "pkg", + "package_root": str(tmp_path), + "exclude": ["AbstractBase"], + } + genpackage.generate_module_per_subpackage(model, layout, overwrite=False) + + assert "'exclude' is only honored" in capsys.readouterr().err + + def test_main_dispatches_genpackage_subcommand(monkeypatch): """`cppwg genpackage ...` routes to genpackage.main with the remaining args.""" captured = {}