Skip to content

Conversation

@PavelBal
Copy link
Member

No description provided.

@PavelBal PavelBal requested a review from Copilot December 16, 2025 12:02
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 adds support for passing arrays of .NET objects as arguments to .NET methods. Previously, only individual NetObject instances could be passed; now R lists or arrays containing NetObject instances can be passed as well, with their pointers automatically extracted.

  • Enhanced argument processing to extract pointers from lists/arrays of NetObject instances
  • Added test coverage for both single and array NetObject arguments
  • Updated documentation to clarify requirements for passing arrays of .NET objects

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
R/rSharp-internal.R Added logic to detect and extract pointers from lists of NetObject instances
tests/testthat/test-net-object.R Added tests verifying single and array NetObject arguments work correctly
vignettes/user-guide.Rmd Documented requirement that array elements must all be NetObject instances
NEWS.md Added changelog entry for array support feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@PavelBal PavelBal requested a review from rwmcintosh December 16, 2025 16:16
return arg != null && arg.GetType() == typeof(TestObject);
}

public static bool ArrayOfObjectsArgument(Object[] arg)
Copy link
Member

Choose a reason for hiding this comment

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

This is obviously going to work, but the issue is that you have a method that will accept a collection of any objects, now it's up to the domain API to iterate through it and make sure they are appropriate, do the type conversions and handle errors.

rSharp cannot help with that because it has no domain knowledge, so it can only perform this kind of conversion for string, int, float etc. That's why you can pass arrays of numbers and strings.

It might be fine to include this in rSharp, but I would not use it for OSPSuite-R. Instead, I would add items to collections one-at-a-time, similar to ConcurrentSimulationRunner

This way rSharp matches method names and types through reflection automatically.

Copy link
Member

Choose a reason for hiding this comment

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

Or find a way to implement this as TestObject[] arg and have rSharp create the array dynamically somehow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes agree for the use in ospsuite. Just wanted to add this functionality here as I already imlemented it.

@Yuri05
Copy link
Member

Yuri05 commented Dec 17, 2025

@PavelBal Could you integrate the fix for #181 in this PR?
Should not be complicated :)

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.

5 participants