-
Notifications
You must be signed in to change notification settings - Fork 13
Fix scoped template bug #180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e688e47
6e918ee
6ef1257
3b66b89
416c23c
1278e49
c4c98d5
f6dc175
53c6702
eb94093
b1041a3
150c095
2b5eaea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -42,13 +42,14 @@ class Typename: | |||||||
| namespaces_name_rule = delimitedList(IDENT, "::") | ||||||||
| rule = ( | ||||||||
| namespaces_name_rule("namespaces_and_name") # | ||||||||
| ).setParseAction(lambda t: Typename(t)) | ||||||||
| ).setParseAction(lambda t: Typename.from_parse_result(t)) | ||||||||
|
|
||||||||
| def __init__(self, | ||||||||
| t: ParseResults, | ||||||||
| name: str, | ||||||||
| namespaces: list[str], | ||||||||
| instantiations: Sequence[ParseResults] = ()): | ||||||||
| self.name = t[-1] # the name is the last element in this list | ||||||||
| self.namespaces = t[:-1] | ||||||||
| self.name = name | ||||||||
| self.namespaces = namespaces | ||||||||
|
|
||||||||
| # If the first namespace is empty string, just get rid of it. | ||||||||
| if self.namespaces and self.namespaces[0] == '': | ||||||||
|
|
@@ -63,12 +64,38 @@ def __init__(self, | |||||||
| self.instantiations = [] | ||||||||
|
|
||||||||
| @staticmethod | ||||||||
| def from_parse_result(parse_result: Union[str, list]): | ||||||||
| def from_parse_result(parse_result: list): | ||||||||
| """Unpack the parsed result to get the Typename instance.""" | ||||||||
| return parse_result[0] | ||||||||
| name = parse_result[-1] # the name is the last element in this list | ||||||||
| namespaces = parse_result[:-1] | ||||||||
| return Typename(name, namespaces) | ||||||||
|
|
||||||||
| def __repr__(self) -> str: | ||||||||
| return self.to_cpp() | ||||||||
| if self.get_template_args(): | ||||||||
| templates = f"<{self.get_template_args()}>" | ||||||||
| else: | ||||||||
| templates = "" | ||||||||
|
|
||||||||
| if len(self.namespaces) > 0: | ||||||||
| namespaces = "::".join(self.namespaces) + "::" | ||||||||
| else: | ||||||||
| namespaces = "" | ||||||||
|
|
||||||||
| return f"{namespaces}{self.name}{templates}" | ||||||||
|
|
||||||||
| def get_template_args(self) -> str: | ||||||||
| """Return the template args as a string, e.g. <double, gtsam::Pose3>.""" | ||||||||
| return ", ".join([inst.to_cpp() for inst in self.instantiations]) | ||||||||
|
|
||||||||
| def templated_name(self) -> str: | ||||||||
| """Return the name without namespace and with the template instantiations if any.""" | ||||||||
| if self.instantiations: | ||||||||
| templates = self.get_template_args() | ||||||||
| name = f"{self.name}<{templates}>" | ||||||||
| else: | ||||||||
| name = self.name | ||||||||
|
|
||||||||
| return name | ||||||||
|
|
||||||||
| def instantiated_name(self) -> str: | ||||||||
| """Get the instantiated name of the type.""" | ||||||||
|
|
@@ -84,8 +111,7 @@ def qualified_name(self): | |||||||
| def to_cpp(self) -> str: | ||||||||
| """Generate the C++ code for wrapping.""" | ||||||||
| if self.instantiations: | ||||||||
| cpp_name = self.name + "<{}>".format(", ".join( | ||||||||
| [inst.to_cpp() for inst in self.instantiations])) | ||||||||
| cpp_name = self.name + f"<{self.get_template_args()}>" | ||||||||
| else: | ||||||||
| cpp_name = self.name | ||||||||
| return '{}{}{}'.format( | ||||||||
|
|
@@ -129,7 +155,7 @@ class BasicType: | |||||||
| rule = (Or(BASIC_TYPES)("typename")).setParseAction(lambda t: BasicType(t)) | ||||||||
|
||||||||
| rule = (Or(BASIC_TYPES)("typename")).setParseAction(lambda t: BasicType(t)) | |
| rule = (Or(BASIC_TYPES)("typename")).setParseAction(BasicType) |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 'lambda' is just a simple wrapper around a callable object. Use that object directly.
| rule = (Typename.rule("typename")).setParseAction(lambda t: CustomType(t)) | |
| rule = (Typename.rule("typename")).setParseAction(CustomType) |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TemplatedType.from_parse_result calls t.template_params.as_list(), but pyparsing.ParseResults exposes asList() (capital "L") rather than as_list(), so this will raise an AttributeError during parsing of templated types. Convert the parse results using asList() or an equivalent (e.g. list(t.template_params)) so that the list of Type objects is built correctly.
| return TemplatedType(t.typename, t.template_params.as_list(), | |
| return TemplatedType(t.typename, list(t.template_params), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@varunagrawal I ran copilot review on your last PR here, and it did flag the issue. Gemini recommended to just use the deprecated call asList which is compatible across versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not correct. Using a deprecated function call is putting a bandage on a fracture and will come back to bite us.
Updating the requirements in the pyproject.toml is the way to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Is that already updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is.
Line 29 in 1740473
| "pyparsing>=3.2.5", |
I don't quite understand why Python/Pip is not respecting the requirement. Will look into it a bit more over the weekend after I finish the Hybrid State Estimation paper. :)
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,8 @@ | |
|
|
||
| import gtwrap.interface_parser as parser | ||
| from gtwrap.template_instantiator.constructor import InstantiatedConstructor | ||
| from gtwrap.template_instantiator.helpers import (InstantiationHelper, | ||
| from gtwrap.template_instantiator.helpers import (InstantiatedMember, | ||
| InstantiationHelper, | ||
| instantiate_args_list, | ||
| instantiate_name, | ||
| instantiate_return_type, | ||
|
|
@@ -57,7 +58,7 @@ def __init__(self, original: parser.Class, instantiations=(), new_name=''): | |
|
|
||
| # Instantiate all instance methods | ||
| self.methods = self.instantiate_methods(typenames) | ||
|
|
||
| self.dunder_methods = original.dunder_methods | ||
|
|
||
| super().__init__( | ||
|
|
@@ -99,9 +100,11 @@ def instantiate_parent_class(self, typenames): | |
| """ | ||
|
|
||
| if isinstance(self.original.parent_class, parser.type.TemplatedType): | ||
| return instantiate_type( | ||
| self.original.parent_class, typenames, self.instantiations, | ||
| parser.Typename(self.namespaces())).typename | ||
| namespaces = self.namespaces() | ||
| typename = parser.Typename(name=namespaces[-1], | ||
| namespaces=namespaces[:-1]) | ||
| return instantiate_type(self.original.parent_class, typenames, | ||
|
Comment on lines
102
to
+106
|
||
| self.instantiations, typename).typename | ||
| else: | ||
| return self.original.parent_class | ||
|
|
||
|
|
@@ -140,7 +143,7 @@ def instantiate_static_methods(self, typenames): | |
|
|
||
| return instantiated_static_methods | ||
|
|
||
| def instantiate_methods(self, typenames): | ||
| def instantiate_methods(self, typenames) -> list[InstantiatedMember]: | ||
| """ | ||
| Instantiate regular methods in the class. | ||
|
|
||
|
|
@@ -225,9 +228,8 @@ def cpp_typename(self): | |
| ", ".join([inst.to_cpp() for inst in self.instantiations])) | ||
| else: | ||
| name = self.original.name | ||
| namespaces_name = self.namespaces() | ||
| namespaces_name.append(name) | ||
| return parser.Typename(namespaces_name) | ||
|
|
||
| return parser.Typename(name=name, namespaces=self.namespaces()) | ||
|
|
||
| def to_cpp(self): | ||
| """Generate the C++ code for wrapping.""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 'lambda' is just a simple wrapper around a callable object. Use that object directly.