Skip to content

Conversation

@9and3
Copy link
Contributor

@9and3 9and3 commented Jan 21, 2024

Hello!

This is the first working draft to port the compas action componentizer workflow to Python 3.x. in Grasshopper v8 (see issue #12). I tested it in CI and on other local projects and it works well, but I can't promise it to be fully bug free :) Looking forward to recive feedback to integrate and thanks again for this great workflow! 🙏

image

🐍🐍🐍

changelog

  • porting action and componentizer from ipy to cpy (pythonnet mainly)
  • some guids changed and added new guids for typehints (extrusion and pointcloud) +modified the old ones for str, float, ghdoc, and none.
  • implemented the new Script logic in componentizer.py
  • the metadata.json is cleaned off the ironpython outdated entries in the serialization and updated with the new ones

discussions

  • This version is serializing all components into CPython components, it does not distinguish python2.x from 3.x. Hence legacy ironpython code will fail in some cases with this action. I wonder if both need to be recognized and serialized differently?
  • not important: I the old out parameter is not shown by default. Especially now with the new ScriptComponent it seems that the problem tab is taking up its role. As a python coder in Rhino I am kinda affectionate to it, so maybe it could be set as an option in metadata.json to wether or not showing it in the component (?).
  • haven't tested modules import (yet)

Copy link
Member

@gonzalocasas gonzalocasas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! This is super cool!
I added a bunch of comments. And there's one thing I'd like to request to change: it might be better for a smoother migration of users already using this action, that we don't remove the support for iropython, but instead add support for cpython, It could be as simple as reverting the changes on componentize.py and creating a componentize-cpytyhon.py with the new support for py3. Then, I guess the action.yml will need to determine which one to run based on some input.

So, by default, this action could still do ironpython, but passing a param, it could switch to build py3 components.

Returns:
result: The sum of all three values.
"""
from ghpythonlib.componentbase import executingcomponent as component
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the legacy ghscript component has a VERY strange internal logic that depends on the class of the component inheriting from something called component. Rhino checked this using a string comparison, not type checking. That's the reason why executingcomponent is renamed here. Is this silly thing still true for the new script component?

Copy link
Contributor Author

@9and3 9and3 Jan 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmm I see. I tried with inheritance directly from executingcomponent and the ghenv.Component.Message stops working although the ghuser component can be built and added to the canvas. If I go back to component everything works as expected.

also @gonzalocasas : I do not know if it is connected but the SDK mode is not working anymore since this was a feature of the old ghpython component to activate with IsAdvancedMode set to True. To my limited extent of knowledge of the new ScriptComponent there is no such feature.

@9and3
Copy link
Contributor Author

9and3 commented Jan 22, 2024

Wow! This is super cool! I added a bunch of comments.

thanks! I am going through them.

And there's one thing I'd like to request to change: it might be better for a smoother migration of users already using this action, that we don't remove the support for iropython, but instead add support for cpython, It could be as simple as reverting the changes on componentize.py and creating a componentize-cpytyhon.py with the new support for py3. Then, I guess the action.yml will need to determine which one to run based on some input.
So, by default, this action could still do ironpython, but passing a param, it could switch to build py3 components.

Got it. I'll give it a go!

@9and3
Copy link
Contributor Author

9and3 commented Jan 23, 2024

Hello!

I added the dual functioning of the action.yml and updated both examples and readme. Technically it should have no conflict with the current version since the parameter to switch between cpython and ironpython is by default to the last one:

- uses: compas-dev/compas-actions.ghpython_components@v2
    with:
      source: components
      target: build
      interpreter: cpython  # optional, defaults to ironpython

I tried to update the readme and add two jobs in the build.yml to build both.

Let me know if there is something to modify!

@9and3 9and3 requested a review from gonzalocasas February 4, 2024 23:54
Copy link
Member

@gonzalocasas gonzalocasas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's looking great! I added some small feedback, after addressing those, we can merge!

@9and3
Copy link
Contributor Author

9and3 commented Feb 6, 2024

Done! I tested it again and all is good from my side. Thanks @gonzalocasas for the feedback, this super nice workflow and for compas y'all🙏

Copy link
Member

@gonzalocasas gonzalocasas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! I'm merging this, updating docs to reference a new v3 v5 release, and releasing it! Thanks a lot!

@gonzalocasas gonzalocasas merged commit 5a75fba into compas-dev:main Feb 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants