-
Notifications
You must be signed in to change notification settings - Fork 4
Adding PlantGL_Mesh method #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Just for information it is possible that windows-latest does not pass because of this issue openalea/lpy#56 in lpy |
Yes that is probably it. Seems that it is a utf-8 encoding issue. |
|
Yes but what is not straightforward is that the file where the error happens has not be changed for 4 or 5 years. And in September 16th the CI was ok. The only thinks that changed since is plantgl. |
src/openalea/widgets/plantgl.py
Outdated
| #plot.colorbar_object_id = randint(0, 1000) | ||
| return plot | ||
|
|
||
| def PlantGL_Mesh(pglobject, plot=None, group_by_color=True, property=None, side='front'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not completly understand.
This is already done by the PlantGL function, isn't it?
Why do you need another function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main difference is that PlantGL return a plot while you return a mesh.
Perhaps, you can extend the to_mesh function...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with the PlantGL function is that it returns a k3d.plot and you can't add two k3d.plot items together.
It's not a problem if you simply want to plot a single scene but in my case, I want to make animate a time series with ipywidgets.
So what I do is compute all the meshes of that series, convert them as k3d.Mesh with this function and add them all to a k3d.plot, make them invisible and have a slider select a time step to display
|
IMHO, this is the role of tomesh function. So rather than having several PlantGL_*, we can extend the tomesh function and document it. |
|
Strange the error is a failed test, but on local windows tests pass. Ok a old windows 10 |
src/openalea/widgets/plantgl.py
Outdated
|
|
||
| def tomesh(geometry, d=None, side='front'): | ||
| """Return a mesh from a geometry object""" | ||
| def tomesh(object, d=None, side='front'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be carefull: object is a reserved keyword in Python.
Either keep geometry but document it.
Or use obj, if you want
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh ! yeah you're right my bad
Adding a new
PlantGL_Meshmethod to make individual meshes as ak3d.Meshobjects.A
k3d.Plotobject, like the ones produced by thePlantGLfunction, cannot be added together.