cellml / libcellml

Repository for libCellML development.
https://libcellml.org
Apache License 2.0
17 stars 21 forks source link

Should bindings exactly follow the C++ code, or be adapted to other language practices? #186

Closed MichaelClerx closed 7 years ago

MichaelClerx commented 7 years ago

Over at https://github.com/MichaelClerx/libcellml/pull/1#issuecomment-327671945 @hsorby and I have been having a discussion about the organisation of generated bindings. We're focussing on Python bindings, but the same arguments will apply for other languages. @hsorby rightly suggested we bring it up here so everyone can have a say.

Should we make the bindings follow the same organisation/structure/naming as the C++ code? This will make it easier to maintain, and means the C++ documentation on code organisation will apply to other bindings (as well as making the examples easier to translate).

On the other hand, people coding in Python tend to prefer code that follows Python guidelines (PEP8) and conventions.

For example, matching the C++ code closely in Python would mean creating lots of modules:

import libcellml
import libcellml.units
import libcellml.model
import libcellml.component

x = libcellml.unit.Units()
y = libcellml.model.Model()
z = libcellml.component.Component()

whereas Python users would expect something more like:

import libcellml

x = libcellml.Units()
y = libcellml.Model()
z = libcellml.Component()

(There are several ways to implement this but that's not the point of this ticket)

There are more aspects to this. For example the name libcellml in C/C++ lingo usually means it's a shared lib(rary) for the project "cellml". If we're going by this logic, the Python module should simply be called cellml.

Going one step further, we could even consider renaming methods from object.camelCased() to object.camel_cased() in Python, but that would probably be a lot of extra work... There is precedent for breaking the underscore-naming rules in big libraries, for example PyQt and PySide both wrap C++ Qt code, change the organisation to match Python standards but leave method names intact. See: http://pyqt.sourceforge.net/Docs/PyQt4/qfiledialog.html On the other hand, libcellml is a much, much smaller project so it's not impossible to do these things by hand.

Thoughts?

nickerso commented 7 years ago

No thoughts yet, but wanting to get @metatoaster's view on this as one of the key targets for python bindings will be PMR.

metatoaster commented 7 years ago

Matching C++ code closely to Python is not exactly an issue, given that it is always possible to have what basically are import aliases on the root namespace. For instance, asyncio, part of the Python Standard Library since 3.4, has many submodules within that module, however it is possible to do from asyncio import Future even though the module lives in asyncio.futures module. Other libraries like sqlalchemy also follow this practice, and I too have many different modules (even packages) for code organisation. It should be possible to have something like this as part of the Python bindings for libcellml.

Whether the Python package is called libcellml or cellml, I don't have a preference. Though libgit2 has theirs as pygit2 but seriously stop with the py-prefixed names, I am already in Python, stop reminding me. Much like the shim example, we could just have a package that exports all the top level imports at Python to a cellml.py which actually contains the names which they imported from the real package as explained earlier. Downside is that cellml will not be able to be a namespace package, but I don't think this will be an issue at all. My recommendation is to pick one and reserve the name now on PyPI.

Oops, I guess I talked about implementation, but end of the day is, I think it isn't bad to anchor the organisation of the code to typical C++ standards, and bridge it to whatever language quirks for their bindings.

As for camelCase or snake_case, I personally have no preference because I've used both styles and seen other projects using both in various Python projects that I've worked with. However, I have come to find snake_case being easier to read because I don't need to randomly parse some uppercase letters in some sentences, even though I started programming with languages that encouraged camelCase (which I also committed a lot of this while programming in Python; won't find that in my newer stuff, however).

One last thing I will say is this: keep the naming and casing consistent. If the policy is that variable names are named like some_variable and method names are to be doSomethingNice, keep it that way. If everything is snake_case I am fine. However, having some methods and/or variables be one way and then another in a different class is something I find appalling.

agarny commented 7 years ago

Maybe one thing to add to what has already been said: if I recall correctly, several years ago, some people from the SBML community complained that libSBML's Java binding wasn't Java-like enough for them. So much so that they ended up writing a pure Java version of libSBML (JSBML?). Needless to say that we shouldn't go to that extent, but we should certainly make sure that whatever binding we come up with is as close to the bound language as possible.

jonc125 commented 7 years ago

I'd be inclined to agree with @metatoaster : keep the naming convention and hierarchy the same as the C++ code, so you can read the C++ docs and figure out what the Python equivalents are easily, but also provide the extra niceties that one would expect from a Python library, such as a cellml top-level package that imports classes from sub-packages for you, docstrings everywhere, etc.

MichaelClerx commented 7 years ago

Shall I try and get cellml on PyPI then?

agarny commented 7 years ago

Personally, I would try to get libCellML. This is how the API is called, not libcellml, CellML, cellml, etc.

hsorby commented 7 years ago

In our case I feel the project is called libcellml and it is just what it is a bit like libxml2. I'm pretty sure @nickerso banned me from using cellml as a namespace or anything on the grounds that it isn't CellML or some such. There was an argument attached to it but at the end of I wasn't allowed to use cellml.

hsorby commented 7 years ago

I'm more in favour of libcellml as this is what I hope we decide the package is called.

mirams commented 7 years ago

I think things tend to be lower case in python? Plus I wouldn't have to remember the capitalisation if we used 'libcellml'. But if this is the only possible, or the most important, 'cellml' you might want to import into python, then that's even simpler?

hsorby commented 7 years ago

I would definitely go lowercase crazy mixed case would lead to just that!

agarny commented 7 years ago

Ok, I am happy with libcellml as a Python package name.

MichaelClerx commented 7 years ago

Cool! Should we ask @metatoaster to claim it on PyPI so that all the passwords etc. are in the same place? Happy to do it myself otherwise

metatoaster commented 7 years ago

https://pypi.python.org/pypi/libcellml

I can grant ownership permissions to whoever should have it. Please let me know.

agarny commented 7 years ago

From https://pypi.python.org/pypi/libcellml: "... as part of the CellML 1.1/1.2 hierarchical model description"...? I imagine it should read "CellML 2.0", if we are to mention a version, or simply "CellML"?

metatoaster commented 7 years ago

Copied verbatim from https://github.com/cellml/libcellml

metatoaster commented 7 years ago

Also, at this stage, we got no actual release yet, that can be fixed when it happens.

MichaelClerx commented 7 years ago

Both should be fixed to show our intentions!

agarny commented 7 years ago

I would use "CellML" to avoid any future 'problem'.

MichaelClerx commented 7 years ago

Can people with permissions modify the README.rst via github? Or do we have to go the whole fork/branch/pull-request route for these things too?

hsorby commented 7 years ago

My philosophy on this would be that typos and minor fixes that are textual are okay to be done inplace. Code changes not so much.

MichaelClerx commented 7 years ago

Could you change it then? Cause I can't :-D

hsorby commented 7 years ago

Nope I'm not privileged in that way. All CellML board members should have permission to do this.

jonc125 commented 7 years ago

It would appear I am privileged - done in f3125cd for develop.

MichaelClerx commented 7 years ago

@agarny @nickerso @jonc125 Could you add me to the list?

nickerso commented 7 years ago

invitation sent.

metatoaster commented 7 years ago

And updated the PyPI listing with the new README.

MichaelClerx commented 7 years ago

Have updated the Python init file so that all classes and functions can be accessed without the submodules (which may be needed for other languages, I guess?)