[Caspar] Fix examples + pybinding PascalCase#458
Conversation
aaron-skydio
left a comment
aaron-skydio
left a comment
There was a problem hiding this comment.
as far as testing, if you've tested the kernel_example and multiple_factors example on your machine, that's good enough for me, the changes here i don't think require testing on multiple machines or configs etc
| from symforce.caspar.examples.kernel_example.generated import ( # type: ignore[import-not-found] | ||
| caspar_lib as lib, | ||
| ) | ||
| lib = caslib.import_lib(out_dir) |
There was a problem hiding this comment.
hmm @emillma was there a reason you'd left this in the previous form with a comment?
There was a problem hiding this comment.
Forgot to mention this, but I could not make that import work.
There was a problem hiding this comment.
Looked more into this. The compiled .so is placed in the source tree at runtime, so its location is not known at type-check time and the import will always appear unresolved to static analysis tools. caslib.import_lib(out_dir) loads the module directly from the known output path
There was a problem hiding this comment.
yeah the typechecker not working was expected, that's why the type: ignore was here. i personally was never really a fan of that pattern and would prefer the version you have here with import_lib, just would rather check with Emil if I'm missing something
There was a problem hiding this comment.
yeah actually i think this can just go ahead, you not being able to get the previous version to work is enough for me to go ahead (but Emil is still welcome to weigh in if he wants)
There was a problem hiding this comment.
As long as the linter and suggestions work with the lib= version I'm all for moving to using this one. The #type: ignore is there because the generated library in not tracked by git iirc.
There was a problem hiding this comment.
I'm getting this error on both python 3.10 and 3.12:
Traceback (most recent call last):
File "/tmp/symforce-test/symforce/caspar/examples/kernel_example/gen_and_run.py", line 48, in <module>
from symforce.caspar.examples.kernel_example.generated import ( # type: ignore[import-not-found]
ModuleNotFoundError: No module named 'symforce.caspar.examples.kernel_example.generated'So I'm suggesting we remove this import method until this issue is investigated. Talked to @emillma about this.
There was a problem hiding this comment.
Sounds good to me. Presumably also applies to bal/gen_and_run.py?
There was a problem hiding this comment.
No, importing using from generated import caspar_libworks as expected.
|
I'll do some more testing just to be sure. I'll keep you posted. |
|
Tested on one more machine. Example code works as expected. |
Nice, thanks |
|
I think then the only remaining thing here is #458 (comment) |
|
All changes tested and examples running as expected. |
3 participants
Seems like some of the examples were left out when migrating to snake case. There was also an instance of snake case in the C++ python bindings which I had to change to make the code run. This fix was tested on just one machine, so an independent test would be nice before a potential merge.