Skip to content
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

Rhino8 scripts #1384

Closed
wants to merge 13 commits into from
Closed

Rhino8 scripts #1384

wants to merge 13 commits into from

Conversation

chenkasirer
Copy link
Member

@chenkasirer chenkasirer commented Aug 14, 2024

The installer will not install using symlinks for CPython in Rhino8. pip shall be used for that.

  • For Rhino8, install compas using symlinks in both IronPython locations (IronPython, IronPython legacy).
  • In no version is specified, install for all available versions.
  • Added the new cpython componentizer to tasks.py (and the required pythonnet to requirements-dev.txt).

What type of change is this?

  • Bug fix in a backwards-compatible manner.
  • New feature in a backwards-compatible manner.
  • Breaking change: bug fix or new feature that involve incompatible API changes.
  • Other (e.g. doc update, configuration, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
  • I ran all tests on my computer and it's all green (i.e. invoke test).
  • I ran lint on my computer and there are no errors (i.e. invoke lint).
  • I added new functions/classes and made them available on a second-level import, e.g. compas.datastructures.Mesh.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate)

Copy link

codecov bot commented Aug 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.24%. Comparing base (4720aaa) to head (a03fa3b).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1384      +/-   ##
==========================================
- Coverage   60.25%   60.24%   -0.02%     
==========================================
  Files         207      207              
  Lines       22244    22244              
==========================================
- Hits        13404    13401       -3     
- Misses       8840     8843       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chenkasirer chenkasirer marked this pull request as ready for review August 14, 2024 15:06

return path


def _get_default_rhino_ironpython_sitepackages_path_mac(version):
def _get_default_rhino_ironpython_sitepackages_path_mac(version, legacy):
# TODO: is there a distinction between legacy and new IronPython components on Mac?
Copy link
Member

Choose a reason for hiding this comment

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

i don't know

CHANGELOG.md Outdated
Comment on lines 79 to 80
* `compas_rhino.install` with no version specified installs to all available versions of Rhino.
* `compas_rhino.install` installs to IronPython, IronPython (Old) and CPython environments.
Copy link
Member

Choose a reason for hiding this comment

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

could you make an overview of what is done by the install command for different versions and scenarios?

  • the phrasing seems to suggest that all these installations are attempted simultaneously.
  • i have not had much success with symlinking and the CPython environment (i think we should promote using pip in that case)

Copy link
Member

Choose a reason for hiding this comment

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

  • having packages installed both in the scripts folder and in the site packages folder breaks the new code editor
  • in the new script editor, users should use only one technique consistently (# r: compas, ... v. [rhinopython] -m pip install compas ...) because the installed packages end up in different locations and will again clash

Copy link
Member Author

Choose a reason for hiding this comment

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

No version specified

(compas-dev) C:\Users\ckasirer\repos\compas>python -m compas_rhino.install 
Install folder for Rhino version 5.0 not found: The scripts folder does not exist in this location: C:\Users\ckasirer\AppData\Roaming\McNeel\Rhinoceros\5.0\scripts
Install folder for Rhino version 6.0 not found: The scripts folder does not exist in this location: C:\Users\ckasirer\AppData\Roaming\McNeel\Rhinoceros\6.0\scripts

Installing COMPAS packages to Rhino 7.0 scripts folder:
C:\Users\ckasirer\AppData\Roaming\McNeel\Rhinoceros\7.0\scripts

   compas               OK
   compas_ghpython      OK
   compas_model         OK
   compas_rhino         OK
   compas_timber        OK
   compas_bootstrapper  OK

Running post-installation steps...

   compas_ghpython      OK: Installed 5 GH User Objects
   compas_timber        OK: Installed 31 GH User Objects

Install completed.

Installing COMPAS packages to Rhino 8.0 scripts folder:
C:\Users\ckasirer/.rhinocode/py27-rh8/Lib/site-packages

   compas               OK
   compas_ghpython      OK
   compas_model         OK
   compas_rhino         OK
   compas_timber        OK
   compas_bootstrapper  OK

Running post-installation steps...

   compas_ghpython      OK: Installed 5 GH User Objects
   compas_timber        OK: Installed 31 GH User Objects

Install completed.

Installing COMPAS packages to Rhino 8.0 scripts folder:
C:\Users\ckasirer/AppData/Roaming/McNeel/Rhinoceros/8.0/scripts

   compas               OK
   compas_ghpython      OK
   compas_model         OK
   compas_rhino         OK
   compas_timber        OK
   compas_bootstrapper  OK

Running post-installation steps...

   compas_ghpython      OK: Installed 5 GH User Objects
   compas_timber        OK: Installed 31 GH User Objects

Install completed.

Only Rhino 7

(compas-dev) C:\Users\ckasirer\repos\compas>python -m compas_rhino.install -v7.0

Installing COMPAS packages to Rhino 7.0 scripts folder:
C:\Users\ckasirer\AppData\Roaming\McNeel\Rhinoceros\7.0\scripts

   compas               OK
   compas_ghpython      OK
   compas_model         OK
   compas_rhino         OK
   compas_timber        OK
   compas_bootstrapper  OK

Running post-installation steps...

   compas_ghpython      OK: Installed 5 GH User Objects
   compas_timber        OK: Installed 31 GH User Objects

Install completed.

Only Rhino8

(compas-dev) C:\Users\ckasirer\repos\compas>python -m compas_rhino.install -v8.0 

Installing COMPAS packages to Rhino 8.0 scripts folder:
C:\Users\ckasirer/.rhinocode/py27-rh8/Lib/site-packages

   compas               OK
   compas_ghpython      OK
   compas_model         OK
   compas_rhino         OK
   compas_timber        OK
   compas_bootstrapper  OK

Running post-installation steps...

   compas_ghpython      OK: Installed 5 GH User Objects
   compas_timber        OK: Installed 31 GH User Objects

Install completed.

Installing COMPAS packages to Rhino 8.0 scripts folder:
C:\Users\ckasirer/AppData/Roaming/McNeel/Rhinoceros/8.0/scripts

   compas               OK
   compas_ghpython      OK
   compas_model         OK
   compas_rhino         OK
   compas_timber        OK
   compas_bootstrapper  OK

Running post-installation steps...

   compas_ghpython      OK: Installed 5 GH User Objects
   compas_timber        OK: Installed 31 GH User Objects

Install completed.

@petrasvestartas
Copy link
Contributor

petrasvestartas commented Aug 23, 2024

I am checking this pull request too for different windows and mac versions.

I am wondering if there is any problems with Rhino8 Grasshopper componentizer.
For .NET development all the plugins needs to be currently and in the future Rhino8, Rhino9 WIP releases be rebuilt from .NET framework to .NET Core to make code execution faster. Is this somehow handled in the current workflow?

But I do not know if this is valid for Python non compiled components.

@tomvanmele
Copy link
Member

let's for now just focus on solving the legacy install problem...

@petrasvestartas
Copy link
Contributor

petrasvestartas commented Aug 23, 2024

@chenkasirer

Okey installation worked, but I am still confused, that conda environment needed ironpython.
Without it, grasshopper components were not built because command invoke build-ghuser-components did not find ipy.
From the current compas2 installation instructions for a new user this is not obvious.
I only knew this because I did it before.

Still checking what is happening in this pull request.

Would it be possible to make an example of bash commands so that I could replicate in a completely new environment without dependencies, just compas?

(base) C:\brg\2_code\compas>conda activate rhino8_scripts

(rhino8_scripts) C:\brg\2_code\compas>git branch
  main
  pr-1384
* rhino8_scripts

(rhino8_scripts) C:\brg\2_code\compas>invoke build-ghuser-components
Cloning into 'C:\Users\petras\AppData\Local\Temp\tmpbg17gtd9actions.ghcomponentizer'...
GHPython componentizer
======================
[x] Source: C:\brg\2_code\compas\src\compas_ghpython\components (5 components)
[x] Target: C:\brg\2_code\compas\src\compas_ghpython\components\ghuser
[x] GH_IO assembly: C:\Users\petras\AppData\Local\Temp\tmp5yy48qr5ghio\GH_IO.dll

Processing component bundles:
  [x] Compas_FromJson => C:\brg\2_code\compas\src\compas_ghpython\components\ghuser\Compas_FromJson.ghuser
  [x] Compas_Info => C:\brg\2_code\compas\src\compas_ghpython\components\ghuser\Compas_Info.ghuser
  [x] Compas_RpcCall => C:\brg\2_code\compas\src\compas_ghpython\components\ghuser\Compas_RpcCall.ghuser
  [x] Compas_ToJson => C:\brg\2_code\compas\src\compas_ghpython\components\ghuser\Compas_ToJson.ghuser
  [x] Compas_ToRhinoGeometry => C:\brg\2_code\compas\src\compas_ghpython\components\ghuser\Compas_ToRhinoGeometry.ghuser

(rhino8_scripts) C:\brg\2_code\compas>python -m compas_rhino.install -v7.0

Installing COMPAS packages to Rhino 7.0 scripts folder:
C:\Users\petras\AppData\Roaming\McNeel\Rhinoceros\7.0\scripts

   compas               OK
   compas_ghpython      OK
   compas_rhino         OK
   compas_bootstrapper  OK

Running post-installation steps...

_____________Installed C:\brg\2_code\compas\src\compas_ghpython\components\ghuser
   compas_ghpython      OK: Installed 5 GH User Objects

Install completed.

(rhino8_scripts) C:\brg\2_code\compas>


V7 invoke build-ghuser-components:
image

V8 invoke build-cpython-ghuser-components:
image

If I use invoke build-ghuser-components and python -m compas_rhino.install on both versions it install ironpython components.

Else-if I use invoke build-cpython-ghuser-components and python -m compas_rhino.install on both versions it install cpython components.

First case breaks installation for V8, since, it is an old component.
Second case breaks installation for V7, since it is CPython.
What command do use for building for both v7 and v8?

So either you need to use this python -m compas_rhino.install -v7.0 or python -m compas_rhino.install -v8.0, but this one:
python -m compas_rhino.install should not be used by the user.


image

Can you handle this dual case, by detecting which Rhino it is, when running single command: python -m compas_rhino.install that when called, would run first the build process for V7 and V8 together ?


according to chen this should conflict with the CPython install because apparently CPython doesn't look in the scripts folder
Rhino 8 looks inside script folder for components which is ok! Just not the packages. These are two separate things.


Other issue, even if we are running ironpython on v8, we still need to build using cpython workflow? Is it correct?
Or the componentizer is setup up differently?


All of this is needed to distribute components, but still we are not solving pip installation issue, when the components are installed.


Shall we ask a Zoom meeting with ScriptEditor developer? I think we all have some questions and requests for him.

@chenkasirer
Copy link
Member Author

@petrasvestartas

Thanks for checking it out so thoroughly.

Regarding the conflict between the user objects themselves: good point.
At least on the building part, one option would be to provide separate configs to the building task here

compas/tasks.py

Lines 29 to 32 in 2d32ec0

"ghuser": {
"source_dir": "src/compas_ghpython/components",
"target_dir": "src/compas_ghpython/components/ghuser",
"prefix": "COMPAS: ",

We could, for example, set separate target dirs for Rhino7 and Rhino8. That way, the built user object shouldn't overwrite each other.

On the installation side of things: difficult.. it's kind of unfortunate Rhino use the same directory for all kinds of components. I guess they did that so that you could sort of seamlessly transition to Rhino8, without all of your plugins disappearing.

One possible solution that pops to mind is adding a pre/suffix to the user object file names. that way they can coexist. How this would look like in Grasshopper, I'm not sure.

According to my calculations ;) in the Rhino 8 there are three types of components which are allowed to coexist:

  • CPython
  • Ironpython
  • Ironpython legacy

The best way to deal with this may depend on what Mcneel are planning to do with GH and these different types.. maybe ironpython is going away soon?

Anyways, our componentizers only produce 2 of these 3 types. To create "modern", Rhino8 components we'd need an additional componentizer (or some flag in the current ironpython one).

Copy link
Member

@tomvanmele tomvanmele left a comment

Choose a reason for hiding this comment

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

LGTM (let's just try and see what happens)

@chenkasirer
Copy link
Member Author

chenkasirer commented Aug 29, 2024

@tomvanmele I notice now something you've been saying all along..
The new script editor has \\AppData\\Roaming\\McNeel\\Rhinoceros\\8.0\\scripts',
So there could indeed be a conflict between packages installed via pip and ones install to the "legacy" path above using symlinks.

I will close this PR for now until we have a better solution. Probably compile "modern" components and installing using symlinks only to the original path (py27/site_packages) is the right way to go.

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.

3 participants