ocaml_python_bindgen icon indicating copy to clipboard operation
ocaml_python_bindgen copied to clipboard

Generated OCaml interface's style

Open UnixJunkie opened this issue 3 years ago • 6 comments

It would be nice if people don't have to put this penultimate unit at the end of a function declaration (in the *_specs.txt file). Once you are in ocaml, having to add those () to call a function doesn't look good. But, I admit it's just a question of style.

UnixJunkie avatar Jan 28 '22 08:01 UnixJunkie

Yeah it is a bit clunky as is.

However, it's currently using the penultimate unit for a couple of things...eg differentiating between Python attributes/properties and instance/class/static methods.

Also, with requiring all of the args to be named (or optional with ?), I just took a shortcut and had the penultimate unit required to avoid the unable to erase errors/warnings. A better way would be to say...check if an optional arg is in the last spot and only then require the unit, but that would require a bit of tweaking.

So, while I agree it's not the typical style, it will probably be around for a bit as I'm working on some other features before this one.

mooreryan avatar Feb 06 '22 20:02 mooreryan

The requiring all args to be named is also questionable in terms of taste. I would have been fine with all args being positional (i.e. in the same order than the Python interface), and no penultimate unit required. Maybe, using a command-line option when the interface is generated would be nice in the end. I.e. choosing between all positional or all named arguments.

UnixJunkie avatar Feb 07 '22 01:02 UnixJunkie

Yeah I think ultimately something like a command line flag to chose would be nice. Or just smarter handling of the args in general would be welcome.

Ideally, I wouldn't want the binding generator to force a certain style on a user, but as of now that is essentially what it's doing.

Personally I prefer labels (eg I open Base on every project), and so that is what I implemented. But I definitely understand that that isn't the style or practice of everyone. So eventually it's something I would like to see addressed.

mooreryan avatar Feb 07 '22 01:02 mooreryan

I was thinking of some ideas on how to address this in a nice way.

The labeled args and the penultimate unit are currently used in certain cases with differentiating between Python attributes/properties, instance methods, class methods, module methods, etc. (As well as ensuring you get erasable args in some cases.) I originally had use of both everywhere so the generated APIs would all be the same, i.e., all have penultimate unit even when not strictly necessary. (And because it was closer to the labels everywhere style I use as I mentioned earlier.)

However, with the latest updates to the main branch, you can add attributes to value specifications (eg, [@@py_fun_name __init__]). So using attributes in certain situations would give a nice way to not have to fall back on the penultimate unit and named args to differentiate anything.

Here are some examples of what I mean.

Avoiding penultimate unit

For example, the way it differentiates between an instance method that takes no arguments and a attribute/property is with the penultimate unit:

val age : t -> string
val name : t -> unit -> string
class Person:
    def __init__(self, name, age):
        self.name = name
        self.age = age

    def name(self):
        return(self.name)

However, with attributes, it could be done like this:

val age : t -> string
[@@py_fun_type attribute]

val name : t -> string

And name would be treated as an instance method as it would be default in this case.

Class methods vs. instance methods

Currently, any value spec that takes a positional t as its first argument is treated as an instance method.

val f : t -> ... -> unit -> ...

You could envision an odd Python class method that actually takes an instance of that class as the first argument. To differentiate, currently pyml_bindgen requires that you name the first argument.

val g : foo:t -> ... -> unit -> ...

In this way it knows that its a class method and not an instance method. However, with attributes, the above could be simplified.

val f : t -> ... -> unit -> ...

val g : t -> ... -> unit -> ...
[@@py_fun_type class_method]

That would get rid of the need for labeled arguments in that case. (I still need to check on if things get weird with the bindings if you mix positional, labeled, and optional arguments...Perhaps in the beginning, you will not be allowed to mix them? I'm not sure yet.)

Non-erasable args

The other place penultimate unit is necessary would be to make sure arguments are erasable.

val f : t -> foo:string -> ?bar:string -> unit -> string

Currently, I enforce this in pyml_bindgen, but I would be open to the idea of letting users add it only when necessary.


Anyway, what do you think of this @UnixJunkie?

mooreryan avatar Feb 09 '22 01:02 mooreryan

I don't follow everything, but here are some comments:

  • the name method in your example should probably return a string.
  • the current pyml_bindgen defaults are quite OK in fact, because at least they are consistent (name all args, always put a penultimate unit) and your thing "just works" (tm).
  • whatever constraint making your life easier is OK. I.e. not allowing to mix positional and named arguments; the user is only allowed to generate an interface where all arguments are either all named or all positional; seems very reasonable to me. I think one key of software engineering is deciding upon such constraints, to make a problem that seems almost intractable become very easy to deal with.
  • about arguments with a default value (if that's what you mean by optional arguments) should not be favored, since that's something bad from the python world that people will not want to creep into their OCaml codebase; in OCaml people prefer a lot to have all parameters being passed explicitly. As an (old?) OCaml programmer, I would be very fine if the generated OCaml interface to some python code makes all parameters become explicit.

UnixJunkie avatar Feb 09 '22 02:02 UnixJunkie

Yeah good catch with the int/string thing...just a typo there. And thanks for the input on your other points as well. Anyway, I will play around with some of these ideas and see how they go.

I will leave the issue open for now and in the meantime, feel free to add any additional thoughts you might have about the matter as they come up.

mooreryan avatar Feb 09 '22 02:02 mooreryan