Repository navigation
Emit the Python device interface as __init__.py #141
Description
Activity
so a device repository has to add a re-export module to recover the name it asked for.
My two cents :P Imo this a feature, not a bug. (See here for an example https://github.com/AllenNeuralDynamics/Aind.Behavior.Services/blob/e4237e2f6ff38922140518fce3ca26015a2c312c/src/aind_behavior_services/rig/harp.py#L5)
I am not aware of a tool that generates under
__init__.py, most often the defaults are used to hint ecossystem (eg. _pb2.py), or the fact that the code is automatically generated (_generated.py). Also, while generators will be used in production to generate full packages, I wouldn't be mad if people use them for static code generation across multiple devices in the same folder. Finally__init__.pymodules are often used as a place where people do all sorts of hacks ("all", conditional imports, etc...), which makes automatic generated code annoying to deal with.@bruno-f-cruz Fair on the conventions.
*_pb2.pyand*_generated.pyboth say something, one naming the ecosystem and the other the provenance, anddevice.pysays neither, so this argues against the current name as much as against__init__.py.The re-export in
aind_behavior_servicesdoes more than undo a file name, since it selects and renames what it re-exports, and this option remains available whichever way the generator emits. What the fixed name forces is the shim that exists only to recover the name the caller already asked for, which is a separate problem.The multi-device case is worth separating out, because the fixed name already prevents it. Generating two devices into one folder overwrites
device.py, since the name is a constant and-ois the only lever, so a directory per device is required today regardless. Under__init__.pythe directory count is the same and the import path loses a segment.PEP 561 is the argument I find hardest to answer. A generated package that downstream code type-checks against needs
py.typed, and a single module inside a PEP 420 namespace package has nowhere to put it. protobuf does not hit this, because its generated modules sit inside an ordinary package that already carries the marker, whereas for a per-device Harp package the generated module is the distribution.The
__all__and conditional-import point is real and is the cost, though smaller than it first looks. Hand-written code already has an established place inside a generated package here.converters.pyis written by hand, sits beside the generated module, is imported by it, and survives regeneration, and the generator emits that import only for schemas that declare a converter outside the standard set. What regeneration owns outright is the package root, so hand-written extensions must remain in a separate module.One genuine constraint the change adds is that a hand-written sibling must not import back into its own package root, since while
__init__.pyis executing the package is only partly initialized.converters.pynever does, because it imports only fromharp.protocol, but nothing stops the next one from doing so, and the error CPython raises for this suggests renaming__init__.pyover a collision with a library name, which is not the cause at all.Where a package wants a hand-written root, pointing
-oat a private subpackage can give you one:src/mypackage/mydevice/__init__.py hand-written, re-exports src/mypackage/mydevice/_interface/__init__.py generated src/mypackage/mydevice/_interface/converters.py hand-written src/mypackage/mydevice/helpers.py hand-written src/mypackage/mydevice/py.typedThe generated relative import resolves against
_interface, which is whyconverters.pymoves down with it.py.typedat the package root covers the subpackage recursively. Andhelpers.pycan import the register classes from._interfacewithout the constraint above, since it runs once that package is fully initialized, so this layout is also where hand-written code that builds on the generated classes belongs. It is your re-export, kept as a choice for the packages that want it rather than a requirement for every package.What any such layout gives up is that the register classes are defined one level down and keep that as their module, so
reprand tracebacks reportmypackage.mydevice._interface.DigitalInputswhile callers import frommypackage.mydevice. This applies to a shim overdevice.pyin the same way, and generating straight into__init__.pyis the only arrangement where the reported module and the import path agree.Which leaves the file name. I still think
__init__.pyis the right default, and the layout above covers most of what an option would have been used for, so I am currently less inclined to add one, but let me know if there is a case it does not cover.What you're describing is a packaging convention that needs more than a file rename to actually work — py.typed, placing the file under src/foo, and a proper pyproject.toml all matter too, and none of those are emitted by the generator. So this proposal makes the "quick" use case (generate a file, import from the root) clunkier, in exchange for a packaged use case it doesn't fully deliver anyway.
For example, take a self-contained script at the repo root following PEP 723. I don't think init.py even works there: from . import RegisterFoo requires relative imports, and relative imports only work when the module is loaded as part of a package afaik.
Bottom line: I'd leave the file name as a non-reserved constant, so it stays usable out of the box for non-packaged scenarios, and handle packaging explicitly (e.g. pass the name via the CLI) for production use. That split seems unavoidable anyway, since you'll always want the file saved into a subfolder rather than the root.
I agree a rename alone does not deliver packaging, and the PEP 723 specifically is much harder to handle with the subpackage layout. I've pushed a change to #143 to add a new
packageoption instead of changing the name. With the option off, which is the default, nothing changes and you get a singledevice.py. With it on, the interface is generated as__init__.pywith apy.typedmarker, so the output directory alone sets the import path.I would prefer to use a flag instead of a free name, because
convertersis already reserved. The generated module already emitsfrom .converters import ..., so a caller passing that name today would get a module that imports from itself, and any file the generator adds later would do the same to whoever had picked its name.pyproject.tomlstays out, since it needs to carry a name and version the generator would have to invent.
The generated file names are constants in every target. That is harmless for the C# and firmware targets, where the file name carries no meaning and everything a consumer names comes from
--namespaceor from the schema. It is not harmless for Python, where the file name is the module name, so the constantdevice.pylands directly in the import path of every generated package.The Python target already treats the output directory as the namespace, and rejects
--namespaceoutright, so-ois the only lever over where the module ends up. A fixed file name then appends a trailing segment no caller can remove. Generating intosrc/harp/behaviorproducesharp.behavior.devicerather thanharp.behavior, so a device repository has to add a re-export module to recover the name it asked for.Emitting
__init__.pyinstead would let the output directory alone determine the import path.That looks better than adding an option for the file name, for three reasons.
The directory already names the module, so an option would introduce a second way to say the same thing, and the two can disagree.
PEP 561 needs a package directory to carry
py.typed. A single module inside a namespace package has nowhere to put the marker, so a generated package that is to be type-checked downstream needs the directory form regardless.Harp Python already hand-places exactly this file.
harp/device/core/__init__.pycarries the generated header, so the core register interface is already a generated module sitting at a package initializer.One cost worth stating plainly. A generated
__init__.pymeans the package root cannot carry hand-written code, because regeneration overwrites it. That looks affordable for a device package, since converters for custominterfaceTypevalues are generated too, but anything hand-written would have to live in a sibling module.This came up while adding a generated Python package to
harp-tech/device.behavior, which under the current naming needs a re-export shim whose only purpose is to undo the file name.