Skip to content

Conversation

@ivaylo-matov
Copy link
Contributor

Purpose

This PR is related to DYN-8088 and should be reviewed along the PythonNet3Engine PR#12 .

  • adds a new Python test that exercises the updated PythonNet3 list decoding behavior.
  • removes the Failure flag from TestListDecoding now that the runtime fix is in place.

DYN-8088-fix

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

Copilot AI review requested due to automatic review settings December 3, 2025 15:11
Copy link

@github-actions github-actions bot left a 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

Copy link
Contributor

Copilot AI left a 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 TestListDecoding test now that the runtime fix is implemented
  • Added TryDecode_ConvertsNestedPythonListsToClrLists test to validate nested list conversion behavior

data = [[1, 2, 3], [4, 5, 6]]
OUT = data, List.Flatten(data, -1)
";
var empty = new ArrayList();
Copy link

Copilot AI Dec 3, 2025

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.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

now addressed

Comment on lines +340 to +341
[Test]
public void TryDecode_ConvertsNestedPythonListsToClrLists()
Copy link

Copilot AI Dec 3, 2025

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.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

now addressed

@ivaylo-matov
Copy link
Contributor Author

ivaylo-matov commented Dec 3, 2025

Build and failing test will pass once PythonNet3 Engine is updated. I've pumber up the PythonNet3 version to 1.4.6

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.

1 participant