Skip to content

Commit

Permalink
Revert #196 (#203)
Browse files Browse the repository at this point in the history
  • Loading branch information
sizmailov authored Nov 27, 2023
1 parent 6cfda36 commit 99f14a1
Show file tree
Hide file tree
Showing 37 changed files with 23 additions and 452 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ Changelog
=========


Version 2.4.2 (Nov 27, 2023)
--------------------------
Changes:
- 🔁 Revert #196 due to poor review


Version 2.4.1 (Nov 25, 2023)
--------------------------
Changes:
Expand Down
22 changes: 1 addition & 21 deletions pybind11_stubgen/parser/errors.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from pybind11_stubgen.structs import Identifier, Import, QualifiedName, Value
from pybind11_stubgen.structs import Identifier, QualifiedName


class ParserError(Exception):
Expand Down Expand Up @@ -33,23 +33,3 @@ def __init__(self, name: QualifiedName):

def __str__(self):
return f"Can't find/import '{self.name}'"


class AmbiguousEnumError(InvalidExpressionError):
def __init__(self, repr_: str, *values_and_imports: tuple[Value, Import]):
super().__init__(repr_)
self.values_and_imports = values_and_imports

if len(self.values_and_imports) < 2:
raise ValueError(
"Expected at least 2 values_and_imports, got "
f"{len(self.values_and_imports)}"
)

def __str__(self) -> str:
origins = sorted(import_.origin for _, import_ in self.values_and_imports)
return (
f"Enum member '{self.expression}' could not be resolved; multiple "
"matching definitions found in: "
+ ", ".join(f"'{origin}'" for origin in origins)
)
4 changes: 0 additions & 4 deletions pybind11_stubgen/parser/mixins/error_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ def handle_method(self, path: QualifiedName, class_: type) -> list[Method]:
with self.__new_layer(path):
return super().handle_method(path, class_)

def finalize(self) -> None:
with self.__new_layer(QualifiedName.from_str("finalize")):
return super().finalize()

@property
def current_path(self) -> QualifiedName:
assert len(self.stack) != 0
Expand Down
142 changes: 5 additions & 137 deletions pybind11_stubgen/parser/mixins/fix.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@
import re
import sys
import types
from collections import defaultdict
from logging import getLogger
from typing import Any, Sequence

from pybind11_stubgen.parser.errors import (
AmbiguousEnumError,
InvalidExpressionError,
NameResolutionError,
ParserError,
Expand Down Expand Up @@ -1001,56 +999,13 @@ def parse_value_str(self, value: str) -> Value | InvalidExpression:


class RewritePybind11EnumValueRepr(IParser):
"""Reformat pybind11-generated invalid enum value reprs.
For example, pybind11 may generate a `__doc__` like this:
>>> "set_color(self, color: <ConsoleForegroundColor.Blue: 34>) -> None:\n"
Which is invalid python syntax. This parser will rewrite the generated stub to:
>>> from demo._bindings.enum import ConsoleForegroundColor
>>> def set_color(self, color: ConsoleForegroundColor.Blue) -> None:
>>> ...
Since `pybind11_stubgen` encounters the values corresponding to these reprs as it
parses the modules, it can automatically replace these invalid expressions with the
corresponding `Value` and `Import` as it encounters them. There are 3 cases for an
`Argument` whose `default` is an enum `InvalidExpression`:
1. The `InvalidExpression` repr corresponds to exactly one enum field definition.
The `InvalidExpression` is simply replaced by the corresponding `Value`.
2. The `InvalidExpression` repr corresponds to multiple enum field definitions. An
`AmbiguousEnumError` is reported.
3. The `InvalidExpression` repr corresponds to no enum field definitions. An
`InvalidExpressionError` is reported.
Attributes:
_pybind11_enum_pattern: Pattern matching pybind11 enum field reprs.
_unknown_enum_classes: Set of the str names of enum classes whose reprs were not
seen.
_invalid_default_arguments: Per module invalid arguments. Used to know which
enum imports to add to the current module.
_repr_to_value_and_import: Saved safe print values of enum field reprs and the
import to add to a module when when that repr is seen.
_repr_to_invalid_default_arguments: Groups of arguments whose default values are
`InvalidExpression`s. This is only used until the first time each repr is
seen. Left over groups will raise an error, which may be fixed using
`--enum-class-locations` or suppressed using `--ignore-invalid-expressions`.
_invalid_default_argument_to_module: Maps individual invalid default arguments
to the module containing them. Used to know which enum imports to add to
which module.
"""

_pybind11_enum_pattern = re.compile(r"<(?P<enum>\w+(\.\w+)+): (?P<value>-?\d+)>")
# _pybind11_enum_pattern = re.compile(r"<(?P<enum>\w+(\.\w+)+): (?P<value>\d+)>")
_unknown_enum_classes: set[str] = set()
_invalid_default_arguments: list[Argument] = []
_repr_to_value_and_import: dict[str, set[tuple[Value, Import]]] = defaultdict(set)
_repr_to_invalid_default_arguments: dict[str, set[Argument]] = defaultdict(set)
_invalid_default_argument_to_module: dict[Argument, Module] = {}

def __init__(self):
super().__init__()
self._pybind11_enum_locations: dict[re.Pattern, str] = {}
self._is_finalizing = False

def set_pybind11_enum_locations(self, locations: dict[re.Pattern, str]):
self._pybind11_enum_locations = locations
Expand All @@ -1069,104 +1024,17 @@ def parse_value_str(self, value: str) -> Value | InvalidExpression:
return Value(repr=f"{enum_class.name}.{entry}", is_print_safe=True)
return super().parse_value_str(value)

def handle_module(
self, path: QualifiedName, module: types.ModuleType
) -> Module | None:
# we may be handling a module within a module, so save the parent's invalid
# arguments on the stack as we handle this module
parent_module_invalid_arguments = self._invalid_default_arguments
self._invalid_default_arguments = []
result = super().handle_module(path, module)

if result is None:
self._invalid_default_arguments = parent_module_invalid_arguments
return None

# register each argument to the current module
while self._invalid_default_arguments:
arg = self._invalid_default_arguments.pop()
assert isinstance(arg.default, InvalidExpression)
repr_ = arg.default.text
self._repr_to_invalid_default_arguments[repr_].add(arg)
self._invalid_default_argument_to_module[arg] = result

self._invalid_default_arguments = parent_module_invalid_arguments
return result

def handle_function(self, path: QualifiedName, func: Any) -> list[Function]:
result = super().handle_function(path, func)

for f in result:
for arg in f.args:
if isinstance(arg.default, InvalidExpression):
# this argument will be registered to the current module
self._invalid_default_arguments.append(arg)

return result

def handle_attribute(self, path: QualifiedName, attr: Any) -> Attribute | None:
module = inspect.getmodule(attr)
repr_ = repr(attr)

if module is not None:
module_path = QualifiedName.from_str(module.__name__)
is_source_module = path[: len(module_path)] == module_path
is_alias = ( # could be an `.export_values()` alias, which we want to avoid
is_source_module
and not inspect.isclass(getattr(module, path[len(module_path)]))
)

if not is_alias and is_source_module:
# register one of the possible sources of this repr
self._repr_to_value_and_import[repr_].add(
(
Value(repr=".".join(path), is_print_safe=True),
Import(name=None, origin=module_path),
)
)

return super().handle_attribute(path, attr)

def report_error(self, error: ParserError) -> None:
# defer reporting invalid enum expressions until finalization
if not self._is_finalizing and isinstance(error, InvalidExpressionError):
if isinstance(error, InvalidExpressionError):
match = self._pybind11_enum_pattern.match(error.expression)
if match is not None:
return
super().report_error(error)

def finalize(self) -> None:
self._is_finalizing = True
for repr_, args in self._repr_to_invalid_default_arguments.items():
match = self._pybind11_enum_pattern.match(repr_)
if match is None:
pass
elif repr_ not in self._repr_to_value_and_import:
enum_qual_name = match.group("enum")
enum_class_str, _ = enum_qual_name.rsplit(".", maxsplit=1)
enum_class_str, entry = enum_qual_name.rsplit(".", maxsplit=1)
self._unknown_enum_classes.add(enum_class_str)
self.report_error(InvalidExpressionError(repr_))
elif len(self._repr_to_value_and_import[repr_]) > 1:
self.report_error(
AmbiguousEnumError(repr_, *self._repr_to_value_and_import[repr_])
)
else:
# fix the invalid enum expressions
value, import_ = self._repr_to_value_and_import[repr_].pop()
for arg in args:
module = self._invalid_default_argument_to_module[arg]
if module.origin == import_.origin:
arg.default = Value(
repr=value.repr[len(str(module.origin)) + 1 :],
is_print_safe=True,
)
else:
arg.default = value
module.imports.add(import_)
super().report_error(error)

def finalize(self):
if self._unknown_enum_classes:
# TODO: does this case still exist in practice? How would pybind11 display
# a repr for an enum field whose definition we did not see while parsing?
logger.warning(
"Enum-like str representations were found with no "
"matching mapping to the enum class location.\n"
Expand Down
2 changes: 1 addition & 1 deletion pybind11_stubgen/parser/mixins/parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def handle_class_member(
def handle_module(
self, path: QualifiedName, module: types.ModuleType
) -> Module | None:
result = Module(name=path[-1], origin=QualifiedName.from_str(module.__name__))
result = Module(name=path[-1])
for name, member in inspect.getmembers(module):
obj = self.handle_module_member(
QualifiedName([*path, Identifier(name)]), module, member
Expand Down
5 changes: 2 additions & 3 deletions pybind11_stubgen/structs.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def parent(self) -> QualifiedName:
return QualifiedName(self[:-1])


@dataclass(eq=False)
@dataclass
class Value:
repr: str
is_print_safe: bool = False # `self.repr` is valid python and safe to print as is
Expand Down Expand Up @@ -110,7 +110,7 @@ class Attribute:
annotation: Annotation | None = field_(default=None)


@dataclass(eq=False)
@dataclass
class Argument:
name: Identifier | None
pos_only: bool = field_(default=False)
Expand Down Expand Up @@ -191,7 +191,6 @@ class Import:
@dataclass
class Module:
name: Identifier
origin: QualifiedName
doc: Docstring | None = field_(default=None)
classes: list[Class] = field_(default_factory=list)
functions: list[Function] = field_(default_factory=list)
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
maintainer_email="sergei.a.izmailov@gmail.com",
description="PEP 561 type stubs generator for pybind11 modules",
url="https://github.com/sizmailov/pybind11-stubgen",
version="2.4.1",
version="2.4.2",
long_description=open("README.md").read(),
long_description_content_type="text/markdown",
license="BSD",
Expand Down
3 changes: 2 additions & 1 deletion tests/check-demo-stubs-generation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ run_stubgen() {
demo \
--output-dir=${STUBS_DIR} \
${NUMPY_FORMAT} \
--ignore-invalid-expressions="\(anonymous namespace\)::(Enum|Unbound)|<demo\._bindings\.flawed_bindings\..*|<ConsoleForegroundColor\\.Magenta: 35>" \
--ignore-invalid-expressions="\(anonymous namespace\)::(Enum|Unbound)|<demo\._bindings\.flawed_bindings\..*" \
--enum-class-locations="ConsoleForegroundColor:demo._bindings.enum" \
--print-safe-value-reprs="Foo\(\d+\)" \
--exit-code
}
Expand Down
8 changes: 0 additions & 8 deletions tests/demo-lib/include/demo/sublibA/ConsoleColors.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,6 @@ enum class ConsoleForegroundColor {
None_ = -1
};

enum class ConsoleForegroundColorDuplicate {
Green = 32,
Yellow = 33,
Blue = 34,
Magenta = 35,
None_ = -1
};

enum ConsoleBackgroundColor {
Green = 42,
Yellow = 43,
Expand Down
6 changes: 5 additions & 1 deletion tests/demo.errors.stderr.txt
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
pybind11_stubgen - [ ERROR] In demo._bindings.aliases.foreign_enum_default : Invalid expression '<ConsoleForegroundColor.Blue: 34>'
pybind11_stubgen - [ ERROR] In demo._bindings.eigen.dense_matrix_c : Can't find/import 'm'
pybind11_stubgen - [ ERROR] In demo._bindings.eigen.dense_matrix_c : Can't find/import 'n'
pybind11_stubgen - [ ERROR] In demo._bindings.eigen.dense_matrix_r : Can't find/import 'm'
pybind11_stubgen - [ ERROR] In demo._bindings.eigen.dense_matrix_r : Can't find/import 'n'
pybind11_stubgen - [ ERROR] In demo._bindings.eigen.four_col_matrix_r : Can't find/import 'm'
pybind11_stubgen - [ ERROR] In demo._bindings.eigen.four_row_matrix_r : Can't find/import 'n'
pybind11_stubgen - [ ERROR] In demo._bindings.enum.accept_defaulted_enum : Invalid expression '<ConsoleForegroundColor.None_: -1>'
pybind11_stubgen - [ ERROR] In demo._bindings.flawed_bindings.accept_unbound_enum : Invalid expression '(anonymous namespace)::Enum'
pybind11_stubgen - [ ERROR] In demo._bindings.flawed_bindings.accept_unbound_enum_defaulted : Invalid expression '<demo._bindings.flawed_bindings.Enum object at 0x1234abcd5678>'
pybind11_stubgen - [ ERROR] In demo._bindings.flawed_bindings.accept_unbound_type : Invalid expression '(anonymous namespace)::Unbound'
pybind11_stubgen - [ ERROR] In demo._bindings.flawed_bindings.accept_unbound_type_defaulted : Invalid expression '<demo._bindings.flawed_bindings.Unbound object at 0x1234abcd5678>'
pybind11_stubgen - [ ERROR] In demo._bindings.flawed_bindings.get_unbound_type : Invalid expression '(anonymous namespace)::Unbound'
pybind11_stubgen - [ ERROR] In finalize : Enum member '<ConsoleForegroundColor.Magenta: 35>' could not be resolved; multiple matching definitions found in: 'demo._bindings.duplicate_enum', 'demo._bindings.enum'
pybind11_stubgen - [WARNING] Enum-like str representations were found with no matching mapping to the enum class location.
Use `--enum-class-locations` to specify full path to the following enum(s):
- ConsoleForegroundColor
pybind11_stubgen - [WARNING] Raw C++ types/values were found in signatures extracted from docstrings.
Please check the corresponding sections of pybind11 documentation to avoid common mistakes in binding code:
- https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-cpp-types-in-docstrings
Expand Down
1 change: 0 additions & 1 deletion tests/py-demo/bindings/src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ PYBIND11_MODULE(_bindings, m) {
bind_classes_module(m.def_submodule("classes"));
bind_eigen_module(m.def_submodule("eigen"));
bind_enum_module(m.def_submodule("enum"));
bind_duplicate_enum_module(m.def_submodule("duplicate_enum"));
bind_aliases_module(m.def_submodule("aliases"));
bind_flawed_bindings_module(m.def_submodule("flawed_bindings"));
bind_functions_module(m.def_submodule("functions"));
Expand Down
1 change: 0 additions & 1 deletion tests/py-demo/bindings/src/modules.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ void bind_aliases_module(py::module&& m);
void bind_classes_module(py::module&& m);
void bind_eigen_module(py::module&& m);
void bind_enum_module(py::module&& m);
void bind_duplicate_enum_module(py::module&& m);
void bind_flawed_bindings_module(py::module&& m);
void bind_functions_module(py::module&& m);
void bind_issues_module(py::module&& m);
Expand Down
15 changes: 0 additions & 15 deletions tests/py-demo/bindings/src/modules/duplicate_enum.cpp

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ from __future__ import annotations
from demo._bindings import (
aliases,
classes,
duplicate_enum,
eigen,
enum,
flawed_bindings,
Expand All @@ -24,7 +23,6 @@ __all__ = [
"aliases",
"classes",
"core",
"duplicate_enum",
"eigen",
"enum",
"flawed_bindings",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ from __future__ import annotations
from . import (
aliases,
classes,
duplicate_enum,
eigen,
enum,
flawed_bindings,
Expand All @@ -21,7 +20,6 @@ from . import (
__all__ = [
"aliases",
"classes",
"duplicate_enum",
"eigen",
"enum",
"flawed_bindings",
Expand Down
Loading

0 comments on commit 99f14a1

Please sign in to comment.