-
Notifications
You must be signed in to change notification settings - Fork 665
[DNM] PythonNet Test #16673
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
[DNM] PythonNet Test #16673
Conversation
This reverts commit 0389294.
…kage-does-not-load
cpython still present but does not load
…com/ivaylo-matov/Dynamo into DYN-9388-Net3-package-does-not-load
…com/ivaylo-matov/Dynamo into DYN-9388-Net3-package-does-not-load
…honNet3-default-in-4.0_251024
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…s://github.com/ivaylo-matov/Dynamo into DYN-9388-Make-PythonNet3-default-in-4.0_251024
This reverts commit 6bde6d0.
Update DynamoCore.csproj
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.
Pull Request Overview
This pull request migrates Python node functionality from CPython3 to PythonNet3 engine within Dynamo. The change switches the default Python engine to PythonNet3, removes the legacy DSCPython library, and adds comprehensive user notification systems for the engine transition.
Key changes include:
- Replacing CPython3 engine references with PythonNet3 throughout the codebase
- Implementing user notifications for automatic engine upgrades
- Updating test files to use PythonNet3 engine instead of CPython3
Reviewed Changes
Copilot reviewed 83 out of 96 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/core/python/*.dyn | Updated Python test graphs to use PythonNet3 engine |
| test/Libraries/DynamoPythonTests/*.cs | Migrated test code from DSCPython to DSPythonNet3 evaluator |
| src/Libraries/DSCPython/ | Removed entire legacy CPython library implementation |
| src/DynamoCoreWpf/Views/Menu/PreferencesView.xaml | Updated preferences UI for PythonNet3 engine reset |
| src/PythonMigrationViewExtension/ | Enhanced migration extension with CPython upgrade notifications |
| src/Libraries/PythonNodeModels/PythonNode.cs | Changed default engine from CPython3 to PythonNet3 |
| src/DynamoCore/Configuration/PreferenceSettings.cs | Added setting to hide CPython upgrade notifications |
Files not reviewed (4)
- src/DynamoCoreWpf/Properties/Resources.Designer.cs: Language not supported
- src/Libraries/DSCPython/Properties/Resources.Designer.cs: Language not supported
- src/Libraries/PythonNodeModels/Properties/Resources.Designer.cs: Language not supported
- src/PythonMigrationViewExtension/Properties/Resources.Designer.cs: Language not supported
Comments suppressed due to low confidence (3)
test/Libraries/DynamoPythonTests/PythonEvalTestsWithLibraries.cs:1
- [nitpick] The comment provides good context for the test quarantine, but could be more specific about the expected timeline for the "parity fix" or reference a tracking issue/ticket for resolution.
test/Libraries/DynamoPythonTests/PythonEditTests.cs:1 - [nitpick] Similar to the previous comment, this quarantine explanation would benefit from more specific details about the expected exception shape differences or a reference to a tracking issue for the parity fix.
test/core/packageDependencyTests/PythonDependency.dyn:1 - The Python dependency test should verify that the engine change from CPython3 to PythonNet3 doesn't break package dependency resolution, but the test file only shows the engine property change without corresponding test logic validation.
| <PackageReference Include="Lucene.Net.QueryParser" Version="4.8.0-beta00016" /> | ||
| <PackageReference Include="DynamoVisualProgramming.Analytics" Version="4.2.2.10200" /> | ||
| <PackageReference Include="DynamoVisualProgramming.PythonEngine.PythonNet3" Version="1.4.3" GeneratePathProperty="true" ExcludeAssets="all" /> | ||
| <PackageReference Include="DynamoVisualProgramming.Analytics" Version="4.2.1.9358" /> |
Copilot
AI
Oct 31, 2025
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 analytics package version was downgraded from 4.2.2.10200 to 4.2.1.9358. This appears to be unintentional and could indicate a dependency conflict that should be investigated.
| <PackageReference Include="DynamoVisualProgramming.Analytics" Version="4.2.1.9358" /> | |
| <PackageReference Include="DynamoVisualProgramming.Analytics" Version="4.2.2.10200" /> |
Purpose
(FILL ME IN) This section describes why this PR is here. Usually it would include a reference to the tracking task that it is part or all of the solution for.
Declarations
Check these if you believe they are true
Release Notes
(FILL ME IN) Brief description of the fix / enhancement. Use N/A to indicate that the changes in this pull request do not apply to Release Notes. Mandatory section
Reviewers
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
(FILL ME IN, optional) Any additional notes to reviewers or testers.
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of