where are the compas.geometry.trimesh* dependencies defined?
Opened this issue · 15 comments
while compas
is in an overall terrific state and a mature library, compas.geomtry.trimesh*
is a glaring exception.
can we consider whether to move it out of the core
lib into a module of its own?
(almost) all of these functions receive their implementation from plugins.
that said, the pluggable/plugin mechanism is in this case certainly not as useful as for example in the case of Brep and Nurbs...
(almost) all of these functions receive their implementation from plugins
right, and there is a neat docstring with no body...
what I find hard to grasp where trimesh
or the plugin is defined.
since trimesh
isn't mentioned in requirements.txt
since trimesh is well maintained, it should be included IMO: now this code just looks too magical.
-
I realise the
plugin
mechanism is there to differentiate fromCPython
vsIronPython
since requirements.txt can be ran conditionally I don't see why these shouldn't be listed? -
why not make the dependency explicit in the docstring? linking to the documentation of the method invoked.
-
the
trimesh_geodistance_numpy
.py file seems missing?
from .trimesh_geodistance_numpy import trimesh_geodesic_distances_numpy
if method == "exact":
raise NotImplementedError
return trimesh_geodesic_distances_numpy(M, [source])
-
it seems that
trimesh*
aren't covered in the test suite? -
same seems to apply to
triangulation_delaunay.py
, whereconstrained_delaunay_triangulation
is core functionality forRhinoVault
IIRC? -
since the root of the
compas.geometry
directory contains the core primitives, it's less then optimal to have so many more (shallow)trimesh*
files in the directory. let's move these to atrimesh
folder and remove thetrimesh
prefix? -
trimesh_parametrisation.py
basically contains 2 methods that raise a NotImplementedError?
the plugins have nothing to do with trimesh
. some of them are defined in compas_cgal
, some in compas_libigl
. the modules are prefixed with trimesh
, because they deal with triangular meshes exclusively.
see here for an example: https://github.com/compas-dev/compas_cgal/blob/dd9f32aade7e81e8c9f941885545dc4042bbec45/src/compas_cgal/slicer.py#L11
i noticed a few broken code paths indeed. have already fixed them and will send a PR.
the explicit registration of plugins for pluggables has been discussed on several occasions in the past. we can start the discussion again based on this thread...
in addition to the explicit registration of the (known) plugins for each pluggable, the goal is to have a default implementation for all of them (ideally in pure Python) that might be slow or less robust but is always available.
One of the main reasons why we don't register known plugins in core is licensing. Adding an explicit registration would put in us in a grey zone in terms of how the plugin's license affects core's license. This is particularly the case with stuff like cgal and igl that have strong copyleft licenses (sometimes paired with a more liberal one in a dual licensing model).
Besides that, I find that from a software architecture point of view, registering a dependency on the "parent's side" creates an ugly circular dependency situation that is not clean to manage.
Thanks for the reference to the example @tomvanmele , that's insightful.
compas_cgal
& compas_libigl
are excellent and I'm very aware of them.
I raised perhaps too many points at once.
I'm curious to have your thoughts on conditional requirements.txt
This seems a biggy to me: it'll allow to state dependencies explicitly.
I think this is key, since it makes the need for the plugin
mechanism explicit too.
stuff like cgal and igl that have strong copyleft licenses
gotcha
registering a dependency on the "parent's side" creates an ugly circular dependency situation that is not clean to manage
I'm not contesting the usefulness of a plugin mechanism. compas.plugin
seems to echo pytest's
[1] plugin architecture to some extent?
[1] any reference to pytest
is a celebration of course ;)
While pip
can define conditional requirements, conda
cannot, so it still wouldn't provide a full solution.
Maybe the simple and good-enough alternative is more documentation
compas.plugin
seems to echopytest's
architecture?
indeed, it's partially inspired by it
(pip)
While pip can define conditional requirements, conda cannot, so it still wouldn't provide a full solution.
environment.yaml
allows for a pip
install block: they're complementary, see here
You basically can defer to a requirements.txt
.
Looking at the selectors that pip
offers, it seems reasonable to assume IronPython
can be detected.
(conda)
conda
definitely supports conditional requirements, not the best reference but see here
See here for the selectors -- so many architectures, except for our "beloved" Iron(y)Python
.
(conda+pip)
Still, deferring to requirements.txt
in a environment.yml
is the way fwd IMO, since otherwise dependencies will be listed twice, which introduces ambiguity. ( compas_cgal
seems to adhere to this mechanism
Ah sorry, I used the wrong term. I didn't mean conditional
but optional
requirements.
So, I thought we were referring to the possibility of doing something like conda install compas[full_including_cgal_and_stuff]
. That optional requirement doesn't work for conda as far as I know.
Creating an environment file definitely allows to fallback to pip, but it doesn't solve the problem because 1) it's not the same as a conda recipe, e.g. we lose the conda install compas
one-liner, 2) it will actually use pip to install stuff, and we use conda exactly to avoid using pip because some stuff is difficult to install (on Windows) using purely pip.
It's technically possible to provide an env file and instruct people to do conda env create -f https://compas.dev/envs/compas_full/environment.yml
but it's not very composable because one cannot have an existing environment and install compas on it that way.
One alternatively would be to create bundle packages, much like ROS does, we could have conda install compas.full.form_finding
or conda install compas.full.robotics
, etc. Those could be simpler meta-packages that just install many other compas package in one go.
That optional requirement doesn't work for conda as far as I know
aha, yes I think so too...
we lose the conda install compas one-liner
by no means I'm challenging conda
: I introduced it to the pythonocc
project many moons ago -- imagine the horrors of building a CAD kernel via pip
.
I don't follow your comment: for compas
dependencies are stated only in requirements.txt
I think the current practice is that conda
is using pip
under the hood as is 💣😉
please see the earlier comment wrt compas_cgal
.
I nice way to work with compas_cgal
is to run conda env update -f=env_mac.yml
followed by pip install -e .
The point in case: there's a soft, malleable boundary between the two tools....
i don't think the conditional requirements help in this case. compas_occ
, compas_cgal
and compas_libigl
cannot be installed with pip
(although this might change). therefore, we can't specify them as optional for a pip
installation.
in the case of conda
we can indeed use an environment file with all sorts of conditionals and additional pip
instructions. this is already the case for packages like compas_cgal
and compas_libigl
. however this doesn't change anything about how compas
or any other package is built on conda-forge
.
if you want compas
in combination with other packages to get some of the plugins in the same environment, you can simply do conda create -n combo compas compas_cgal compas_occ compas_libigl
.
perhaps the solution is to simply reference the plugins from the docs, clearly stating that their installation and related licensing is the responsibility of the user, and add an environment file to the compas
repo that allows users to install compas
with all available plugins if they want, for convenience.
perhaps the solution is to simply reference the plugins from the docs, clearly stating that their installation and related licensing is the responsibility of the user, and add an environment file to the
compas
repo that allows users to installcompas
with all available plugins if they want, for convenience.
+1
perhaps the solution is to simply reference the plugins from the docs, clearly stating that their installation and related licensing is the responsibility of the user, and add an environment file to the compas repo that allows users to install compas with all available plugins if they want, for convenience
sorry for all the technicalities, yes that is the point, to have a path that in CPython
its easy to get a complete installation.
apologies, it took a bit to grasp the circularity of the interdependencies