Skip to content

Conversation

@gangatp
Copy link
Collaborator

@gangatp gangatp commented Sep 2, 2025

C# Binding Fix:

  • Add 'public' modifier to Wrapper class in buildbindingcsharp.go
  • Fixes accessibility issue where Wrapper class was internal instead of public
  • Aligns C# bindings with other languages that properly expose wrapper classes

Python Binding Enhancement:

  • Refactor out parameter handling in buildbindingpython.go
  • Separate 'out' and 'return' parameter processing logic
  • Add optional input parameters for out parameters with default None values
  • Improve backwards compatibility by maintaining out parameters in return values

@gangatp gangatp requested a review from Copilot September 2, 2025 07:00
@gangatp gangatp changed the base branch from master to develop September 2, 2025 07:00
Copy link

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 fixes binding issues for both C# and Python in the Automatic Component Toolkit (ACT). The changes improve C# wrapper class visibility and enhance Python out parameter handling for better backwards compatibility.

  • Adds 'public' modifier to C# Wrapper class to fix accessibility issues
  • Refactors Python out parameter handling to support optional input parameters with default None values
  • Maintains backward compatibility in Python bindings while improving parameter processing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@spywo
Copy link

spywo commented Sep 5, 2025

LGTM

preCallLines = append(preCallLines, fmt.Sprintf("%s = %s(len(%s))", cParams[0].ParamName, cParams[0].ParamCallType, param.ParamName))
preCallLines = append(preCallLines, fmt.Sprintf("%s = (%s*len(%s))(*%s)", cParams[1].ParamName, cParams[1].ParamCallType, param.ParamName, param.ParamName))
pythonInParams = pythonInParams + param.ParamName
inParamsSig = append(inParamsSig, param.ParamName)
Copy link
Member

Choose a reason for hiding this comment

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

formatting seems off

@martinweismann
Copy link
Member

looks good now, except for some incorrect indentation in Source/buildbindingpython.go.
I would recommend running the new version against a large ACT-component, like lib3mf, to see whether the new python bindings are truly backwards compatible to the old ones.

@gangatp
Copy link
Collaborator Author

gangatp commented Sep 22, 2025

Hi @martinweismann, I've fixed the indentations. Also, checked the changes in lib3mf; the example scripts work without any issues. Here are the differences in Lib3mf.py 3MFConsortium/lib3mf@1ab0f8d

Copy link
Member

@martinweismann martinweismann left a comment

Choose a reason for hiding this comment

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

looks good now!

@gangatp gangatp merged commit 794460e into develop Sep 25, 2025
17 checks passed
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.

4 participants