search

compas-dev / compas_fab

Robotic fabrication package for the COMPAS Framework.
https://compas.dev/compas_fab/
MIT License
108 stars 32 forks source link

over elaborate __init__ files are a performance hinderance #410

Open jf--- opened 2 months ago

jf--- commented 2 months ago

I have a small gripe with compas_fab over elaborate __init__.py files.

A specific example is compas_fab/backends/__init__.py

All of the backends are loaded, where you're most likely using a single one at the time. Now, ROS, the analytic solver and pybullet are loaded even while its unlikely to use more one solver at the time. ( FWIW, I'm working on the tesseract backend ) Loading pybullet will take seconds to load... and in the case of macOs pop-up a UI...

So a huge amount of unnecessary code is loaded...

Finally, the __init__ arguable could be just be an empty file, rather then the 166 lines it is now. The "hardcoded" ( dare I say Javaesque 👺 ) style of the current init files are also stylistically a little substandard of the usual finesse and elegance of the compas_fab codebase.

( And then the irony of providing a LazyLoader in compas_fab.utils.lazy_loader 🧐 😉 )

gonzalocasas commented 2 months ago

You are so very right. The lazy loader should actually avoid pybullet from loading, it seems it's not working correctly :/ I've considered multiple times whether it would be a better idea to split off all backends, and have compas_fab_ros, compas_fab_pybullet, etc. They could still live together in this repo for the time being, but it would create a better separation of concerns for sure, and would potentially open up the option of completely moving each backend to a new project/repo in the future without breaking changes in the namespacing.

@jf--- what do you think?

jf--- commented 2 months ago

You are so very right

Confirming my pedantry is an invitation for moar 😉

actually avoid pybullet from loading

I haven't checked that actually

whether it would be a better idea to split off all backends, and have compas_fab_ros, compas_fab_pybullet, etc

a git submodule might make sense here? pybullet definitely qualifies as a plugin

now ( compas 2.0 ) feels like a nice time to do so

yck011522 commented 2 months ago

Regarding splitting the code to a separate repo and link with submodule, I think it would only make sense to spend that energy if we take the opportunity to let users choose which backends to install.

However, I think this is not yet the right time. I think most compas_fab users are beginners into this field, otherwise they would interface with a backend directly, so focusing them to install all and thus going them easy access to try them all is a nice thing.

Moreover each of our backends are not really fully featured yet. And it is much easier to develop / PR / review while keeping everything in one place.