-
Notifications
You must be signed in to change notification settings - Fork 665
DYN-8088 DSCore.List.Flatten does not work in PythonNet3 #16761
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
base: master
Are you sure you want to change the base?
Conversation
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.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8088
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 PR addresses DYN-8088, fixing a PythonNet3 issue where DSCore.List.Flatten did not work correctly with Python lists. The fix removes the failure flag from an existing test that was quarantined due to the list interop issue, and adds a new comprehensive test to verify that nested Python lists are properly converted to CLR lists.
Key Changes
- Removed quarantine flags from
TestListDecodingtest now that the runtime fix is implemented - Added
TryDecode_ConvertsNestedPythonListsToClrListstest to validate nested list conversion behavior
| data = [[1, 2, 3], [4, 5, 6]] | ||
| OUT = data, List.Flatten(data, -1) | ||
| "; | ||
| var empty = new ArrayList(); |
Copilot
AI
Dec 3, 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.
[nitpick] The variable name 'empty' is misleading since it's used as an argument to pythonEvaluator but doesn't reflect its purpose. Consider renaming to something like 'emptyInputs' or 'emptyParameters' to clarify its role in the evaluator call.
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.
now addressed
| [Test] | ||
| public void TryDecode_ConvertsNestedPythonListsToClrLists() |
Copilot
AI
Dec 3, 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.
Consider adding a test case that verifies the behavior when List.Flatten is called with different depth parameters (e.g., 0, 1) to ensure the fix handles various flattening levels correctly, not just the complete flatten with -1.
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.
now addressed
|
Build and failing test will pass once PythonNet3 Engine is updated. I've pumber up the PythonNet3 version to 1.4.6 |
Purpose
This PR is related to DYN-8088 and should be reviewed along the PythonNet3Engine PR#12 .
Declarations
Check these if you believe they are true
Release Notes
Adds a new python test and removes a failure flag.
Reviewers
@zeusongit
@DynamoDS/eidos
FYIs
@dnenov