Issue 174 #175
Issue 174 #175
Conversation
There was a problem hiding this comment.
Hi & thanks for the PR! Looks great to me. A bunch of tests fail but that seems to have more to do with some change to numpy which I need to investigate. I left one comment regarding the new docstring example you added. If you can address that, I'm happy to go ahead and merge.
| -------- | ||
| >>> import navis | ||
| >>> n = navis.example_neurons(1, kind="mesh") | ||
| >>> n_sm = simplify_mesh_blender(n, |
There was a problem hiding this comment.
I think it's a good idea to provide some example here. Currently, this will cause an error during doctests because Blender is not installed on the Github worker.
You can either edit test-package.yml to install Blender before running pytest, or add the # doctest: +SKIP directive to exclude this line from the tests (see other such examples throughout the codebase). In theory, the former should be pretty straight forward: wget the linux distribution, unzip somewhere and add to PATH. In practice, Github Actions can be a pain in the neck to debug.
There was a problem hiding this comment.
Thanks, that's a great idea.
I had issue with the current installation with some mesa related libraries and removed graphviz-dev and mesa-vulkan-drivers from my test-package.yml. I also added the latest blender-LTS via moguri/setup-blender@v1 action.
I did not see the tests finish, but in at least one of the runs I saw the test from b3d pass. Let me know if this could work for you?
There was a problem hiding this comment.
I updated the solution to keeping the ubuntu libraries as they were before and installing the blender directly via wget, not relying on an external and potentially unnecessary complex action. The simplify_mesh_blender seems to pass more reliably now, the comparison between basic types and numpy types still fails, so the overall tests currently still fail.
Let me know if I should change anything else?
* install blender for mesh simplification test
- Install blender directly - add path to environment - make simplify_mesh_blender PASS
|
Looks good to me! Thanks for setting up tests! |
potential fix for issue with blender 4.3:
bpy.ops.import_mesh.stl()tobpy.ops.wm.stl_import()created context error which was fixed by:bpy.ops.object.select_all→bpy.context.scene.objectsDeprecation warning for context was added in blender-3.2. The
wm.stl_importshould work at least since blender-4.0.