Default profile for generator differs?

[kerimoyle]

The generator's profile changes depending on whether you call libcellml in Python or in C++... is this deliberate? I'd thought that the C profile was the default throughout?

In Python:

    generator = Generator()
    generator.processModel(model)
    imp = generator.implementationCode() # <<<< gives output in Python

In C++:

    auto generator = libcellml::Generator::create();
    generator->processModel(model);
    auto imp = generator->implementationCode(); // <<<<< gives output in C

[agarny]

I was surprised, but this is apparently deliberate. I didn't write the Python bindings for the generator (I imagine @hsorby did), but I can see here that, from Python, the default generator profile is indeed for Python. I guess it makes sense when using libCellML from Python.

[kerimoyle]

Hmmm... OK. Not sure when that happened? I guess I'll change the tutorials again then :|

[agarny]

That was done eight months ago (see here).

[kerimoyle]

Yes, but I'd thought it was a bug that was fixed: see https://github.com/cellml/libcellml/issues/508

[agarny]

I had forgotten about it, but as you can see, issue #508 was closed in favour of issue #510 and it's waiting for @hsorby's feedback on what should be done about it, if anything.

[kerimoyle]

Ah, yeah, ditto #600 too. I guess that's why I've had to change stuff backwards ...

[kerimoyle]

Is there any update on how the Python bindings can be expected to behave? ie: create(name) methods in or our? Defaults for generator profile? I need to update the tutorials (again ...) but don't want to do it more times than I have to ... @hsorby @agarny

[agarny]

As far as I know / am concerned, the create() methods are really C++ specific and its different signatures really only for convenience. So, when it comes to Python, I would just have the default constructor and that is it. That's the view I have taken for PR #631 for instance.

Regarding the default generator profile (in Python, I imagine?), we currently have to distinguish between a GeneratorProfile object created directly and one created through the Generator class. In the former case, the generator profile will be for C while in the latter case it will be fore Python. I must confess that I find that a bit confusing. Personally, I would just stick to the true default, i.e. C, no matter how a GeneratorProfile object is created and independently of the language used, i.e. C++ or Python.

I appreciate that when using libCellML from Python, we may want to generate Python by default, but... then what about creating a GeneratorProfile object by "hand"? That one will use a C profile.

On a side note, our Python binding is not Pythonic enough in my view. For instance, the Generator class has a setProfile() method. Such a method name is fine in C++, but in Python you would expect that method to be called set_profile(). Is that something that we could "support" using SWIG? If so, we ought to create an issue for it.

[kerimoyle]

As far as I know / am concerned, the create() methods are really C++ specific and its different signatures really only for convenience. So, when it comes to Python, I would just have the default constructor and that is it. That's the view I have taken for PR #631 for instance.

@agarny This is in direct contradiction to what you said in #510 (below) ... I'm trying to make the tutorials consistent, and now that it seems that the bindings overload the named constructor method, I want to make sure that no future PRs will remove that again.

Thinking more about it, I feel like we need to replace the default constructors that we use in our bindings with the create() method. Indeed, some classes (e.g. Variable) has several versions of the create() method. So, we want to make sure that, say, Python users can access all of them in the same way that C++ users can. @nickerso? @hsorby? @kerimoyle?

... any thoughts?

[agarny]

I was thinking about the create() method without any arguments when I made that first comment.

However, we do have create() methods that have some arguments. This is the case in our Issue class where we have Issue::create(), Issue::create(const ComponentPtr &component), Issue::create(const ImportSourcePtr &importSource), etc. So, when it comes to the interface, we define Issue() (through our create_constructor() method), but also "manually" define Issue(const ComponentPtr &component), Issue(const ImportSourcePtr &importSource), etc. as we rightly should.

I remember checking our codebase while working on issue #499 and was happy with our interfaces reflecting our use of the create() method (be it with or without arguments).