Open-Systems-Pharmacology / OSPSuite-R

R package for the OSPSuite
https://www.open-systems-pharmacology.org/OSPSuite-R/
Other
29 stars 12 forks source link

Allow changing of the `name` property of `Simulation` object #1245

Open PavelBal opened 1 year ago

PavelBal commented 1 year ago

When creating multiple simulations from the same pkml file and saving them, it would be useful to be able to change the Name property, so when loading the simulation in MoBi it will have a name defined in R.

Is this property read-only in .NET and do you see any problems with changing it from R @msevestre @Yuri05

PavelBal commented 11 months ago

@msevestre @Yuri05 @rwmcintosh Is it something that can be easily implemented in Core? For V12?

msevestre commented 11 months ago

I don’t see a problem here We always said that .net object are immutable in terms of name because of path resolution etc But in terms of simulation name, that should probably just work

rwmcintosh commented 11 months ago

I'm looking at this OSPSuite.R.Domain.Simulation class and it has settable name property which is just the underlying ModelCoreSimulation name property.

Is there something else that has to change?

Can someone point to where this should change for me?

Yuri05 commented 11 months ago

I think there is nothing else to change. But it will not check for naming rules violation:

https://github.com/Open-Systems-Pharmacology/OSPSuite.Core/blob/096e3bc4683555594aeb3d767d2a614e1d1d47a3/src/OSPSuite.Presentation/DTO/NonEmptyNameDTO.cs#L23-L45

grafik

rwmcintosh commented 11 months ago

Those checks are part of OSPSuite.Presentation which is not a dependency of OSPSuite.R (it only depends on Core layers below Presentation).

I think it should be handled by R to check for special characters, what do you think?

Yuri05 commented 11 months ago

I think it should be handled by R to check for special characters, what do you think?

I think that's OK. But I would retrieve the list of illegal characters via OSPSuite.R (it's defined in Core and should be accessible)

Yuri05 commented 11 months ago

Actually the name check could be done in OSPSuite.R instead of in R (or maybe that's what you meant?)

rwmcintosh commented 11 months ago

I mean in OSPSuite-R (which is R code), and not OSPSuite.R (which is in C#). But either one will work of course.

Is there input validation already in this R project?

Yuri05 commented 11 months ago

I think no.. but not 100% sure

msevestre commented 11 months ago

This needs to be done in .net side IMO

rwmcintosh commented 11 months ago

Then silently decline to set the property if we find one of these characters?

      public string Name
      {
         get => CoreSimulation.Name;
         set => CoreSimulation.Name = value;
      }
PavelBal commented 11 months ago

Is there input validation already in this R project?

No.

Then silently decline to set the property if we find one of these characters?

I would not do anything silently. Throw an error - it should be caught on the R side.