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

Python Modules #13728

Merged
merged 12 commits into from
Feb 21, 2023
Merged

Python Modules #13728

merged 12 commits into from
Feb 21, 2023

Conversation

dnenov
Copy link
Collaborator

@dnenov dnenov commented Feb 6, 2023

Purpose

This PR complements an update to pythonnet. The update includes a number of useful Python modules now available to the end user straight from Dynamo. The initial list of modules is as follows:

  • numpy
  • pandas
  • pillow
  • scypi
  • matplotlib
  • openpyxl
    - openifcshell IfcOpenShell was removed due to issues with the licensing type

This PR added the updated pythonnet .dll library files:

  • Python.Included.dll
  • Python.Runtime.dll
    - Python.Module.IfcOpenShell

The documentation of pythonnet README file has been updated to include the steps taken to add new modules. A dedicated test is provided for each module, asserting that it is functional. An initial environment manual test has been run with python-3.9.12-embed-amd64 deleted asserting that Dynamo will correctly create the necessary infrastructure to run Python Scripts and the newly added modules.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

Release Notes

This PR contains the following elements to complement pythonnet update.

  • updated .dll files
  • module license information
  • module tests

Reviewers

@sm6srw

FYIs

@Amoursol

- added python modules back to the license file after merge issue
- added back a missing closing tag to DSCPython.csproj
Copy link
Contributor

@sm6srw sm6srw left a comment

Choose a reason for hiding this comment

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

Remove all files that lives in node_modules directories from the PR and delete these files locally. These are leftovers from a recent change where we started to use npm pack instead of npm install . Same goes for any package.json and package-lock.json files.

@dnenov
Copy link
Collaborator Author

dnenov commented Feb 7, 2023

Remove all files that liives in node_modules directories from the PR and delete these files locally. These are leftovers from a recent change where we started to use npm pack instead of npm install . Same goes for any package.json and package-lock.json files.

In \DynamoCoreWpf folder, correct?

- removed package-lock.json and package.json as instructed
@sm6srw
Copy link
Contributor

sm6srw commented Feb 7, 2023

Remove all files that liives in node_modules directories from the PR and delete these files locally. These are leftovers from a recent change where we started to use npm pack instead of npm install . Same goes for any package.json and package-lock.json files.

In \DynamoCoreWpf folder, correct?

It looks like the ones left are in src/Notifications/node_modules. About 100 files that should be removed from this PR.

Copy link
Contributor

@sm6srw sm6srw left a comment

Choose a reason for hiding this comment

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

It also looks like the wrong version of Python.Module.IfcOpenShell.dll is included. Should be 2.5.2.7185 but is `2.5.2.7126'.

@sm6srw sm6srw added DNM Do not merge. For PRs. WIP labels Feb 7, 2023
@sm6srw
Copy link
Contributor

sm6srw commented Feb 7, 2023

The tests are still failing consistently in the pipeline. I can reproduce sporadically locally. Something is still unstable and needs a closer look (something I will do when I am back again next week).

- removed 2 modules that are not going to be used at this moment from the license.txt (already removed from License.rtf)
@Amoursol
Copy link
Contributor

@dnenov can we please remove the "IfcOpenShell" module from this PR? Unfortunately due to the LGPL/GPL license on it we are unable to ship it with Dynamo 😢

@dnenov
Copy link
Collaborator Author

dnenov commented Feb 13, 2023

Ah, that's unfortunate .. . Of course, I will do it! Would it be possible to wait for @sm6srw 's input, as we dedicated a new assembly for this particular module, because of its size? We have to roll back a few things to get it out of the frame. I am mostly certain of the course we will take, but would like to run it by him if that's OK.

- IfcOpenShell not being used due to license type
- updated with the correct .dll from the updated pythnonet
- removed IfcOpenShell test
LICENSE.txt Outdated Show resolved Hide resolved
doc/distrib/License.rtf Outdated Show resolved Hide resolved
src/Libraries/DSCPython/DSCPython.csproj Outdated Show resolved Hide resolved
test/core/python/python_modules/test.ifc Outdated Show resolved Hide resolved
src/Libraries/DSCPython/DSCPython.csproj Outdated Show resolved Hide resolved
src/Libraries/DSCPython/DSCPython.csproj Outdated Show resolved Hide resolved
- any leftover code from the implementation of IfcOpenShell module has been removed
@sm6srw sm6srw removed DNM Do not merge. For PRs. WIP labels Feb 21, 2023
@sm6srw sm6srw merged commit 161e369 into DynamoDS:master Feb 21, 2023
dariaivanciucova pushed a commit to dariaivanciucova/Dynamo that referenced this pull request Apr 13, 2023
* Python Modules Tests added

- tests
- dlls

* Upstream Master Merge

* Update to License.rtf

- added python modules back to the license file after merge issue

* Missing closing tag added back

- added back a missing closing tag to DSCPython.csproj

* Removed package-lock.json and package.json

- removed package-lock.json and package.json as instructed

* Further removed files

* Update pythonnet

- updated dll files

* Removed unused modules from license.txt

- removed 2 modules that are not going to be used at this moment from the license.txt (already removed from License.rtf)

* IfcOpenShell removed due to license issues

- IfcOpenShell not being used due to license type
- updated with the correct .dll from the updated pythnonet
- removed IfcOpenShell test

* Removed any IfcOpenShell-related code

- any leftover code from the implementation of IfcOpenShell module has been removed
dariaivanciucova added a commit to dariaivanciucova/Dynamo that referenced this pull request Apr 13, 2023
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