symforce icon indicating copy to clipboard operation
symforce copied to clipboard

Generated `__init__.py` with multiple functions takes the last

Open aaron-skydio opened this issue 4 years ago • 6 comments

A minimal example:

from symforce import codegen
from symforce import typing as T

def a(x: T.Scalar):
        return x + 1

def b(x: T.Scalar):
        return x + 2

output_dir = "<something>"
codegen.Codegen.function(a, config=codegen.PythonConfig()).generate_function(output_dir=output_dir)
codegen.Codegen.function(b, config=codegen.PythonConfig()).generate_function(output_dir=output_dir)

Then output_dir/python/symforce/sym/__init__.py will contain:

# -----------------------------------------------------------------------------
# This file was autogenerated by symforce from template:
#     python_templates/function/__init__.py.jinja
# Do NOT modify by hand.
# -----------------------------------------------------------------------------

from .b import b

aaron-skydio avatar Jan 17 '22 22:01 aaron-skydio

bumping this. I'm working around this in a hacky way by clearing the init of the target directory and just importing the functions directly into my class (without using the codegen_util.load_generated_package function). is there a more elegant/proper way?

lowjunen avatar Jun 13 '22 22:06 lowjunen

Some thoughts on this:

  1. Generating an __init__.py like this, that imports all generated functions, is going to require some statefulness. For instance, we could read the existing __init__.py in the directory and parse out packages that it's importing and just add to them. Or we could list the contents of the directory and import everything we find. Either way, we're saying that the rendering of a template depends on rendered results of other templates, or stuff on disk, which is a sort of dependency we haven't had to deal with and will get even more thorny if we try to move towards allowing generation of stuff without writing to disk like in #171 .

  2. It's sort of bad practice anyway to import things in __init__.py

For these reasons I'm leaning towards just generating an empty __init__.py (probably could just not generate one, except that py2 requires it). And then of course we'd fix codegen_util.load_generated_package to work correctly with that.

aaron-skydio avatar Jun 14 '22 20:06 aaron-skydio

yeah empty __init__.py definitely feels right. i'm also not using codegen_util.load_generated_package. I'm guessing this is useful for setup the code to work with the Values class? Perhaps my grasp on its benefits is off but I find it's a little overkill for what I'm trying to do (i want to generate a set of functions from the full state dynamics of a quad). Ideally the functions should all be in one file. Perhaps we could add this as an optional argument to generate_function?

lowjunen avatar Jun 15 '22 23:06 lowjunen

codegen_util.load_generated_package is mostly useful if you're primarily interested in generating python code just to run it immediately, as opposed to putting it somewhere you can import from normally (e.g. somewhere on your PYTHONPATH). It sounds like it might be what you want to use if that's what you're trying to do (once we fix this issue), but if you're just trying to generate the python code and then use it e.g. from another python program, yeah you'll want to just import it normally rather than use load_generated_package.

An option to put multiple functions in a single python file (or C++ header, for that matter) is separately a good idea as well I think

aaron-skydio avatar Jun 16 '22 00:06 aaron-skydio

understood. Is there anything i can do to contribute to these? The readme sounds supportive of community driven contributions.

lowjunen avatar Jun 24 '22 01:06 lowjunen

Absolutely! You would be welcome to tackle this and submit a PR, we're happy to provide some guidance too if that's something you'd like to do

aaron-skydio avatar Jun 24 '22 02:06 aaron-skydio