Skip to content

Commit b1e3d5a

Browse files
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__`.
1 parent 63ad07b commit b1e3d5a

File tree

17 files changed

+113
-63
lines changed

17 files changed

+113
-63
lines changed

gen/python/sym/__init__.py

+3-16
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gen/python/sym/_init.py

+21
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

symforce/codegen/backends/python/python_config.py

+8-4
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,16 @@ class PythonConfig(CodegenConfig):
3333
times.
3434
reshape_vectors: Allow rank 1 ndarrays to be passed in for row and column vectors by
3535
automatically reshaping the input.
36+
namespace_package: Generate the package as a namespace package, meaning it can be split
37+
across multiple directories.
3638
"""
3739

3840
doc_comment_line_prefix: str = ""
3941
line_length: int = 100
4042
use_eigen_types: bool = True
4143
use_numba: bool = False
4244
reshape_vectors: bool = True
45+
namespace_package: bool = True
4346

4447
@classmethod
4548
def backend_name(cls) -> str:
@@ -50,10 +53,11 @@ def template_dir(cls) -> Path:
5053
return CURRENT_DIR / "templates"
5154

5255
def templates_to_render(self, generated_file_name: str) -> T.List[T.Tuple[str, str]]:
53-
return [
54-
("function/FUNCTION.py.jinja", f"{generated_file_name}.py"),
55-
("function/__init__.py.jinja", "__init__.py"),
56-
]
56+
templates = [("function/FUNCTION.py.jinja", f"{generated_file_name}.py")]
57+
if self.namespace_package:
58+
return templates + [("function/namespace_init.py.jinja", "__init__.py")]
59+
else:
60+
return templates + [("function/__init__.py.jinja", "__init__.py")]
5761

5862
def printer(self) -> CodePrinter:
5963
return python_code_printer.PythonCodePrinter()

symforce/codegen/backends/python/templates/function/__init__.py.jinja

-1
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,3 @@
22
# SymForce - Copyright 2022, Skydio, Inc.
33
# This source code is under the Apache 2.0 license found in the LICENSE file.
44
# ---------------------------------------------------------------------------- #}
5-
from .{{ spec.name }} import {{ spec.name }}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{# ----------------------------------------------------------------------------
2+
# SymForce - Copyright 2022, Skydio, Inc.
3+
# This source code is under the Apache 2.0 license found in the LICENSE file.
4+
# ---------------------------------------------------------------------------- #}
5+
__path__ = __import__("pkgutil").extend_path(__path__, __name__) # type: ignore # mypy issue #1422
6+
{% if pkg_namespace == "sym" %}
7+
from ._init import *
8+
{% endif %}

symforce/codegen/cam_package_codegen.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,8 @@ def generate(config: CodegenConfig, output_dir: str = None) -> str:
293293
all_types=list(geo_package_codegen.DEFAULT_GEO_TYPES) + list(DEFAULT_CAM_TYPES),
294294
numeric_epsilon=sf.numeric_epsilon,
295295
),
296-
output_path=cam_package_dir / "__init__.py",
296+
output_path=cam_package_dir
297+
/ ("_init.py" if config.namespace_package else "__init__.py"),
297298
)
298299

299300
for name in ("cam_package_python_test.py",):

symforce/codegen/codegen.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ def generate_function(
484484
# Namespace of this function + generated types
485485
self.namespace = namespace
486486

487-
template_data = dict(self.common_data(), spec=self)
487+
template_data = dict(self.common_data(), spec=self, pkg_namespace=namespace)
488488
template_dir = self.config.template_dir()
489489

490490
backend_name = self.config.backend_name()

symforce/codegen/geo_package_codegen.py

+7-1
Original file line numberDiff line numberDiff line change
@@ -217,14 +217,20 @@ def generate(config: CodegenConfig, output_dir: str = None) -> str:
217217
)
218218

219219
# Package init
220+
if config.namespace_package:
221+
templates.add(
222+
template_path=Path("function", "namespace_init.py.jinja"),
223+
data=dict(pkg_namespace="sym"),
224+
output_path=package_dir / "__init__.py",
225+
)
220226
templates.add(
221227
template_path=Path("geo_package", "__init__.py.jinja"),
222228
data=dict(
223229
Codegen.common_data(),
224230
all_types=DEFAULT_GEO_TYPES,
225231
numeric_epsilon=sf.numeric_epsilon,
226232
),
227-
output_path=package_dir / "__init__.py",
233+
output_path=package_dir / ("_init.py" if config.namespace_package else "__init__.py"),
228234
)
229235

230236
# Test example

symforce/opt/numeric_factor.py

+1-4
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,7 @@ def from_file_python(
7575
"""
7676
assert all(opt_key in keys for opt_key in optimized_keys)
7777
function_dir = Path(output_dir) / "python" / "symforce" / namespace
78-
linearization_function = getattr(
79-
codegen_util.load_generated_package(f"{namespace}.{name}", function_dir),
80-
name,
81-
)
78+
linearization_function = codegen_util.load_generated_function(name, function_dir)
8279
return cls(
8380
keys=keys, optimized_keys=optimized_keys, linearization_function=linearization_function
8481
)

test/symforce_codegen_test.py

+52-21
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,10 @@ def test_codegen_python(self) -> None:
143143
"""
144144
inputs, outputs = self.build_values()
145145

146+
config = codegen.PythonConfig(namespace_package=False)
147+
146148
python_func = codegen.Codegen(
147-
inputs=inputs, outputs=outputs, config=codegen.PythonConfig(), name="python_function"
149+
inputs=inputs, outputs=outputs, config=config, name="python_function"
148150
)
149151
shared_types = {
150152
"values_vec": "values_vec_t",
@@ -163,7 +165,7 @@ def test_codegen_python(self) -> None:
163165
actual_dir=output_dir, expected_dir=os.path.join(TEST_DATA_DIR, namespace + "_data")
164166
)
165167

166-
geo_package_codegen.generate(config=codegen.PythonConfig(), output_dir=output_dir)
168+
geo_package_codegen.generate(config=config, output_dir=output_dir)
167169

168170
geo_pkg = codegen_util.load_generated_package(
169171
"sym", os.path.join(output_dir, "sym", "__init__.py")
@@ -200,9 +202,11 @@ def test_codegen_python(self) -> None:
200202

201203
big_matrix = np.zeros((5, 5))
202204

203-
gen_module = codegen_util.load_generated_package(namespace, codegen_data.function_dir)
205+
python_function = codegen_util.load_generated_function(
206+
"python_function", codegen_data.function_dir
207+
)
204208
# TODO(nathan): Split this test into several different functions
205-
(foo, bar, scalar_vec_out, values_vec_out, values_vec_2D_out) = gen_module.python_function(
209+
(foo, bar, scalar_vec_out, values_vec_out, values_vec_2D_out) = python_function(
206210
x,
207211
y,
208212
rot,
@@ -218,6 +222,28 @@ def test_codegen_python(self) -> None:
218222
self.assertStorageNear(foo, x ** 2 + rot.data[3])
219223
self.assertStorageNear(bar, constants.epsilon + sf.sin(y) + x ** 2)
220224

225+
def test_return_geo_type_from_generated_python_function(self) -> None:
226+
"""
227+
Tests that the function (returning a Rot3) generated by codegen.Codegen.generate_function()
228+
with the default PythonConfig can be called.
229+
When test was created, if you tried to do this, the error:
230+
AttributeError: module 'sym' has no attribute 'Rot3'
231+
would be raised.
232+
"""
233+
234+
def identity() -> sf.Rot3:
235+
return sf.Rot3.identity()
236+
237+
output_dir = self.make_output_dir("sf_test_return_geo_type_from_generated_python_function")
238+
239+
codegen_data = codegen.Codegen.function(
240+
func=identity, config=codegen.PythonConfig()
241+
).generate_function(output_dir=output_dir)
242+
243+
gen_identity = codegen_util.load_generated_function("identity", codegen_data.function_dir)
244+
245+
gen_identity()
246+
221247
def test_matrix_order_python(self) -> None:
222248
"""
223249
Tests that codegen.Codegen.generate_function() renders matrices correctly
@@ -243,10 +269,12 @@ def matrix_order() -> sf.M23:
243269
func=matrix_order, config=codegen.PythonConfig()
244270
).generate_function(namespace=namespace, output_dir=output_dir)
245271

246-
pkg = codegen_util.load_generated_package(namespace, codegen_data.function_dir)
272+
gen_matrix_order = codegen_util.load_generated_function(
273+
"matrix_order", codegen_data.function_dir
274+
)
247275

248-
self.assertEqual(pkg.matrix_order().shape, m23.SHAPE)
249-
self.assertStorageNear(pkg.matrix_order(), m23)
276+
self.assertEqual(gen_matrix_order().shape, m23.SHAPE)
277+
self.assertStorageNear(gen_matrix_order(), m23)
250278

251279
def test_matrix_indexing_python(self) -> None:
252280
"""
@@ -270,9 +298,11 @@ def gen_pass_matrices(use_numba: bool, reshape_vectors: bool) -> T.Any:
270298
output_names=["row_out", "col_out", "mat_out"],
271299
).generate_function(namespace=namespace, output_dir=output_dir)
272300

273-
pkg = codegen_util.load_generated_package(namespace, generated_files.function_dir)
301+
genned_func = codegen_util.load_generated_function(
302+
"pass_matrices", generated_files.function_dir
303+
)
274304

275-
return pkg.pass_matrices
305+
return genned_func
276306

277307
def assert_config_works(
278308
use_numba: bool,
@@ -502,7 +532,6 @@ def test_sparse_output_python(self) -> None:
502532
argument of codegen.Codegen.__init__ is set appropriately.
503533
"""
504534
output_dir = self.make_output_dir("sf_test_sparse_output_python")
505-
namespace = "sparse_output_python"
506535
x, y, z = sf.symbols("x y z")
507536

508537
def matrix_output(x: sf.Scalar, y: sf.Scalar, z: sf.Scalar) -> T.List[T.List[sf.Scalar]]:
@@ -514,11 +543,13 @@ def matrix_output(x: sf.Scalar, y: sf.Scalar, z: sf.Scalar) -> T.List[T.List[sf.
514543
name="sparse_output_func",
515544
config=codegen.PythonConfig(),
516545
sparse_matrices=["out"],
517-
).generate_function(namespace=namespace, output_dir=output_dir)
546+
).generate_function(namespace="sparse_output_python", output_dir=output_dir)
518547

519-
pkg = codegen_util.load_generated_package(namespace, codegen_data.function_dir)
548+
sparse_output_func = codegen_util.load_generated_function(
549+
"sparse_output_func", codegen_data.function_dir
550+
)
520551

521-
output = pkg.sparse_output_func(1, 2, 3)
552+
output = sparse_output_func(1, 2, 3)
522553

523554
self.assertIsInstance(output, sparse.csc_matrix)
524555
self.assertTrue((output.todense() == matrix_output(1, 2, 3)).all())
@@ -557,14 +588,14 @@ def numba_test_func(x: sf.V3) -> sf.V2:
557588
output_function = numba_test_func_codegen_data.function_dir / "numba_test_func.py"
558589
self.compare_or_update_file(expected_code_file, output_function)
559590

560-
gen_module = codegen_util.load_generated_package(
561-
"sym", numba_test_func_codegen_data.function_dir
591+
gen_func = codegen_util.load_generated_function(
592+
"numba_test_func", numba_test_func_codegen_data.function_dir
562593
)
563594

564595
x = np.array([1, 2, 3])
565-
y = gen_module.numba_test_func(x)
596+
y = gen_func(x)
566597
self.assertTrue((y == np.array([[1, 2]]).T).all())
567-
self.assertTrue(hasattr(gen_module.numba_test_func, "__numba__"))
598+
self.assertTrue(hasattr(gen_func, "__numba__"))
568599

569600
# -------------------------------------------------------------------------
570601
# C++
@@ -1072,7 +1103,7 @@ def test_function_dataclass(dataclass: TestDataclass1, x: sf.Scalar) -> sf.V3:
10721103
func=test_function_dataclass, config=codegen.PythonConfig()
10731104
)
10741105
dataclass_codegen_data = dataclass_codegen.generate_function()
1075-
gen_module = codegen_util.load_generated_package(
1106+
gen_func = codegen_util.load_generated_function(
10761107
"test_function_dataclass", dataclass_codegen_data.function_dir
10771108
)
10781109

@@ -1083,7 +1114,7 @@ def test_function_dataclass(dataclass: TestDataclass1, x: sf.Scalar) -> sf.V3:
10831114
dataclass_t.v2.v0 = 1
10841115

10851116
# make sure it runs
1086-
gen_module.test_function_dataclass(dataclass_t, 1)
1117+
gen_func(dataclass_t, 1)
10871118

10881119
@slow_on_sympy
10891120
def test_function_explicit_template_instantiation(self) -> None:
@@ -1140,11 +1171,11 @@ class MyDataclass:
11401171
)
11411172

11421173
# Make sure it runs
1143-
gen_module = codegen_util.load_generated_package(namespace, codegen_data.function_dir)
1174+
gen_func = codegen_util.load_generated_function(name, codegen_data.function_dir)
11441175
my_dataclass_t = codegen_util.load_generated_lcmtype(
11451176
namespace, "my_dataclass_t", codegen_data.python_types_dir
11461177
)()
1147-
return_rot = gen_module.codegen_dataclass_in_values_test(my_dataclass_t)
1178+
return_rot = gen_func(my_dataclass_t)
11481179
self.assertEqual(return_rot.data, my_dataclass_t.rot.data)
11491180

11501181

test/symforce_databuffer_codegen_test.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ def gen_code(self, output_dir: str) -> None:
7070
)
7171

7272
# Also test that the generated python code runs
73-
gen_module = codegen_util.load_generated_package(
73+
buffer_func = codegen_util.load_generated_function(
7474
"buffer_func",
7575
py_codegen_data.function_dir,
7676
)
@@ -81,7 +81,7 @@ def gen_code(self, output_dir: str) -> None:
8181
# 2 * buffer[b^2 - a^2] + (a+b)
8282
# 2 * buffer[3] + 3
8383
expected = 9
84-
result_numeric = gen_module.buffer_func(buffer_numeric, a_numeric, b_numeric)
84+
result_numeric = buffer_func(buffer_numeric, a_numeric, b_numeric)
8585

8686
self.assertStorageNear(expected, result_numeric)
8787

Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# -----------------------------------------------------------------------------
22
# This file was autogenerated by symforce from template:
3-
# function/__init__.py.jinja
3+
# function/namespace_init.py.jinja
44
# Do NOT modify by hand.
55
# -----------------------------------------------------------------------------
66

7-
from .codegen_dataclass_in_values_test import codegen_dataclass_in_values_test
7+
__path__ = __import__("pkgutil").extend_path(__path__, __name__) # type: ignore # mypy issue #1422

test/symforce_function_codegen_test_data/symengine/codegen_python_test_data/python/symforce/codegen_python_test/__init__.py

-2
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,3 @@
33
# function/__init__.py.jinja
44
# Do NOT modify by hand.
55
# -----------------------------------------------------------------------------
6-
7-
from .python_function import python_function
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# -----------------------------------------------------------------------------
22
# This file was autogenerated by symforce from template:
3-
# function/__init__.py.jinja
3+
# function/namespace_init.py.jinja
44
# Do NOT modify by hand.
55
# -----------------------------------------------------------------------------
66

7-
from .buffer_func import buffer_func
7+
__path__ = __import__("pkgutil").extend_path(__path__, __name__) # type: ignore # mypy issue #1422
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# -----------------------------------------------------------------------------
22
# This file was autogenerated by symforce from template:
3-
# function/__init__.py.jinja
3+
# function/namespace_init.py.jinja
44
# Do NOT modify by hand.
55
# -----------------------------------------------------------------------------
66

7-
from .codegen_dataclass_in_values_test import codegen_dataclass_in_values_test
7+
__path__ = __import__("pkgutil").extend_path(__path__, __name__) # type: ignore # mypy issue #1422

test/symforce_function_codegen_test_data/sympy/codegen_python_test_data/python/symforce/codegen_python_test/__init__.py

-2
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,3 @@
33
# function/__init__.py.jinja
44
# Do NOT modify by hand.
55
# -----------------------------------------------------------------------------
6-
7-
from .python_function import python_function
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# -----------------------------------------------------------------------------
22
# This file was autogenerated by symforce from template:
3-
# function/__init__.py.jinja
3+
# function/namespace_init.py.jinja
44
# Do NOT modify by hand.
55
# -----------------------------------------------------------------------------
66

7-
from .buffer_func import buffer_func
7+
__path__ = __import__("pkgutil").extend_path(__path__, __name__) # type: ignore # mypy issue #1422

0 commit comments

Comments
 (0)