-
Notifications
You must be signed in to change notification settings - Fork 149
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
JavaScript backend core functionality #159
base: main
Are you sure you want to change the base?
Conversation
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.
High-level looks nice, will hold off on full review until it's more complete (looks based on util.jinja like it's not quite ready yet?)
…' into javascript-backend-core
This fixes Python not being able to properly generate functions that accept 1D and 2D matrix arguments.
@aaron-skydio this is ready for a pass now |
doc_comment_line_prefix: str = " * " | ||
line_length: int = 100 | ||
use_eigen_types: bool = True | ||
# NOTE(hayk): Add JS autoformatter |
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.
Probably prettier
? Which unfortunately doesn't seem to be on PyPI
behavior for codegen compatibility and efficiency. | ||
""" | ||
|
||
def __init__(self, settings: T.Dict[str, T.Any] = None) -> None: |
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.
T.Mapping
instead of T.Dict
""" | ||
|
||
def __init__(self, settings: T.Dict[str, T.Any] = None) -> None: | ||
settings = dict(settings or {},) |
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.
No trailing comma?
symforce/codegen/template_util.py
Outdated
elif extension == "ts": | ||
return FileType.TYPESCRIPT | ||
elif extension == "js": | ||
return FileType.JAVASCRIPT |
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.
Probably leave typescript in here, unless we also delete it from the enum above?
) | ||
|
||
def test_javascript_codegen(self) -> None: | ||
for config in (codegen.PythonConfig(), codegen.CppConfig(), codegen.JavascriptConfig()): |
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.
Is the reasoning for generating it for all three just for debug purposes?
@@ -0,0 +1,52 @@ | |||
// ----------------------------------------------------------------------------- |
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 should probably go in symforce_function_codegen_test_data
? And use the pattern in the other codegen tests so it automatically generates on both backends
3715c7d
to
5874834
Compare
Adds a javascript codegen backend to symforce. This change supports 1D and 2D matrices as javascript Array and Array of Arrays. It generates functions that run in node. It does not yet support geo, cam, or struct types.