-
Notifications
You must be signed in to change notification settings - Fork 150
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Make sym and other genned pkgs namespace packages
Namespace packages are packages whose `__path__` (which tells the python import system to look for sub-packages and sub-modules) has multiple directories, meaning it can have portions spread out across multiple directories. Previously, `sym` and the other generated packages were not namespace packages. This caused issues when generated python packages in the `sym` namespace attempted to access, for example, `sym.Rot3` (which they might do if the generated function returns a `sym.Rot3`). Since the generated package itself was named `sym`, but `Rot3` was not defined locally, an `AttributeError` would be raised. While an alternative would have been to instead not use the name `sym` for generated packages (using, say, `sym_gen` instead), for reasons I didn't fully look into, we still want to generate our code within the `sym` namespace (generating into `sym.gen` was considered as an option but to do so would require the changes in this commit regardless). While currently we haven't needed to generate any functions returning a `sym` class in a package named `sym`, we intend to soon add a `sym.util` package which will be used in a lot of places. That can't be done until this namespace conflict is resolved. Note, while a python2 compatible namespace package has multiple `__init__.py` files for the top-level package spread across different locations, only one of them will be executed. This makes it difficult to re-export the contents of sub-modules into the top-level namespace. The normal way to re-export a name is to write ``` python3 from .sub_module import name ``` However, since the sub-modules can be created dynamically, it is impossible to re-export all names in this manner, as the first `__init__.py` that is created has no way of knowing what names one might want to re-export from subsequent modules. It is possible to put all names one wishes to export in a standard file, say `_init.py`, then dynamically search for such files and execute their contents, but we considered the additional complexity to be too large of a burden (as users would have a harder time understand their generated code, and this would give future maintainers a hard time). And so, we decided to simply stop re-exporting any names in the `__init__.py`'s of generated code (kind of in the style of pep 420 python3 packages). This makes loading a generated function more difficult if one uses `codegen_util.load_generated_package`, as now simply importing a generated package won't give you access to any of the package's contents. However, this is what `codegen_util.load_generated_function` is for, so hopefully the user experience shouldn't be too negatively impacted. The one exception to the general ban of re-exporting names is the `sym` package, as we still wish to be able to do ``` python3 import sym sym.Rot3.identity() ``` However, because all sub-modules we wish to export from the `sym` package are known at code-gen time, allowing this is not difficult. This only applies to names in the core `sym` package, and any additional user generated code in the `sym` package will not be re-rexported in the top-level namespace. A user can prevent their package from being generated as a namespace package by setting the `namespace_package` field of their `PythonConfig` to `False`. This is useful in our testing as it is the generated code being tested that is imported, not, for example, the checked in `sym` package code which is being imported. As a last note, I believe `pkgutil.extend_path` only checks for portions of the package on the `sys.path`, and doesn't check for any portions than can only be found by finders on the `sys.meta_path` (for example, `symforce` itself is found by a finder on the `sys.meta_path` but not on the `sys.path` during an editable pip install). I don't expect this lapse to pose a problem, and addressing it immediately might just make the `__init__.py`s look more complicated than they need to be, but if this does become a problem, know that the situation can be partially addressed trying to find the spec using the finders, and adding the spec's `submodule_search_locations` if found to the `__path__`.
- Loading branch information
1 parent
7f74579
commit 855d587
Showing
18 changed files
with
132 additions
and
64 deletions.
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
16 changes: 16 additions & 0 deletions
16
symforce/codegen/backends/python/templates/function/namespace_init.py.jinja
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
{# ---------------------------------------------------------------------------- | ||
# SymForce - Copyright 2022, Skydio, Inc. | ||
# This source code is under the Apache 2.0 license found in the LICENSE file. | ||
# ---------------------------------------------------------------------------- #} | ||
{% if pkg_namespace == "sym" %} | ||
""" | ||
Python runtime geometry package. | ||
""" | ||
|
||
{% endif %} | ||
# Make package a namespace package by adding other portions to the __path__ | ||
__path__ = __import__("pkgutil").extend_path(__path__, __name__) # type: ignore[has-type] | ||
# https://github.com/python/mypy/issues/1422 | ||
{% if pkg_namespace == "sym" %} | ||
from ._init import * | ||
{% endif %} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 changes: 4 additions & 2 deletions
6
.../symengine/codegen_dataclass_in_values_test_data/python/symforce/codegen_test/__init__.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
# ----------------------------------------------------------------------------- | ||
# This file was autogenerated by symforce from template: | ||
# function/__init__.py.jinja | ||
# function/namespace_init.py.jinja | ||
# Do NOT modify by hand. | ||
# ----------------------------------------------------------------------------- | ||
|
||
from .codegen_dataclass_in_values_test import codegen_dataclass_in_values_test | ||
# Make package a namespace package by adding other portions to the __path__ | ||
__path__ = __import__("pkgutil").extend_path(__path__, __name__) # type: ignore[has-type] | ||
# https://github.com/python/mypy/issues/1422 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.