search

pinterface / burgled-batteries

A bridge between Python and Lisp (FFI bindings, etc.)
Other
113 stars 21 forks source link

Callbacks and other things #6

Open mmontone opened 9 years ago

mmontone commented 9 years ago

Hello!

I've been doing some work on Burgled Batteries, specially regarding callbacks.

Can you review my changes and discuss?

So, this is what I've done:

(defpymodule lisp)
(in-pymodule lisp)
(defpycallback my-callback)

And then module "lisp" is available from Python, and lisp.my_callback can be invoked.

(run* #p"/test.py")
(require :burgled-batteries-demo)
(todo-list:tk)
(todo-list:wx)

Callbacks implementation is not perfect; can be improved; I am trying some ideas, that's why I want to discuss with you.

Thanks!

Mariano

pinterface commented 9 years ago

Dude, could you be a little less awesome? You're making me look bad! :P But seriously, thanks for kicking ass, man.

I will look at this in depth this week and try to get back to you by the end of the weekend at the latest. Some /really minor/ things pop out with a quick perusal:

But like I said, that's all just really minor surface-level stuff. I'm gonna have to spend some time with a caffeinated beverage and a lisp environment to really dig in to your work so I can offer intelligent feedback.

mmontone commented 9 years ago

On 16/06/15 17:32, pinterface wrote:

Dude, could you be a little less awesome? You're making me look bad! :P But seriously, thanks for kicking ass, man.

hahah nahh. You are real guru here! With all that cffi stuff. I'm just trying to build on your work.

I will look at this in depth this week and try to get back to you by the end of the weekend at the latest. Some /really minor/ things pop out with a quick perusal:

  • Watch the tabs, please. This is lisp, we indent with spaces. (If you use emacs, you may want to set indent-tabs-mode to nil for lisp files)

Oh. I was not aware of that. I'm using emacs. I'll set indent-tabs-mode to nil from now on.

  • I prefer uninterned symbols (#:foo) for stuff like defpackage and in-package forms.

Fair enough.

  • Dude, give yourself the author credit for the burgled-batteries-demo system. You wrote it, you deserve the credit!

Ok!

  • What's with the parts that look like copy-pasted macroexpansions? Is there some macro or set of macros you found yourself needing but decided not to include?

I used burgled-batteries.syntax to generate the Python code. The generated code may not look very nice, but it was handy.

But like I said, that's all just really minor surface-level stuff. I'm gonna have to spend some time with a caffeinated beverage and a lisp environment to really dig in to your work so I can offer intelligent feedback.

Great! Looking forward to that.

Cheers

pinterface commented 9 years ago

Okay, proper review time! My apologies for failing to meet my own deadline.

Overall: This is a fantastic start! So much good stuff here!

This response is long, but mostly because it contains a lot of me thinking out loud (you asked for a discussion!), so if you think I'm wrong somewhere, do feel free to argue. :)

Functions Dealing with FILE* (fopen, PyRun_AnyFile, etc.)

These should probably use a new FILE-POINTER type or similar (maybe even just call it FILE*), rather than a raw :POINTER type. Firstly so it's more self-documenting (:pointer is unhelpfully vague), and secondly for future expansion.

E.g., so in the future we have the option of enabling something like:

(with-open-file (f path)
  (run.any-file f path))

Which seems kind of farcical to me, but it's what the cpython guys built for their API.

DEFPYCALLBACK

It might be a good idea for us to figure out what exactly the low-level and high-level DEFPYCALLBACK syntaxes should be, particularly around argument specification. I've thrown some ideas in below, but you're actually writing callbacks so I'd definitely like to hear your thoughts / ideas.

Low-level (PYTHON.CFFI package)

I'd recommend throwing an error if &optional or &rest are encountered, since neither are presently supported.

Positional and &key arguments should probably be bound regardless of whether return-type is :pointer. It seems silly to make people deal with the pain of argument unpacking just because they're also willing to deal with pointers.

Syntax Ideas

Take strings with python names instead of symbols by default, much like the low-level DEFPYFUN. Something like:

(defpycallback name return-type ("arg1" ... &key "keyword_arg" ...)
  ;; here arg1 and keyword-arg are bound as variables
  )

With, of course, the ability to specify the lisp name, if desired. Possibly by just replacing the keyword parameter specifier in an ordinary lambda list[1] and replacing it with a string for the python name:

(defpycallback name return-type
    (;; required parameter with separate python and lisp names
     ("arg1" arg-1)
     ;; key arg with python name, lisp name, default value, and
     ;; supplied-p variable
     &key (("keyword_arg" key-arg) 'default key-arg-supplied?))
  ;; here arg-1, key-arg, and key-arg-supplied? are bound as variables
  )

[1] http://clhs.lisp.se/Body/03_da.htm

Aaand I left the types out of that syntax idea, so obviously that needs some refinement. CL's keyword specifications are surprisingly complicated--I'm very open to ideas on how to represent the full range of possibilities, plus deal with CFFI types, in a way that isn't incredibly painful for people trying to write callbacks. (I'm definitely not asking you to make supplied-p parameters and default values and all that jazz work, I'd just like to have the syntax nailed down so if you or me or somebody else tries to implement those later we haven't painted ourselves into a corner.)

High-level (PYTHON package)

The default return-type should probably not be :pointer. After all, the point of the high-level API is to deal with pointers for you as much as possible. Not sure if we could reasonably default it to OBJECT, or if it would make sense to require the user always specify the return-type.

(... :python-name "foo" :return-type bar) feels a bit verbose. Would it make any sense to compact it down a wee bit? Maybe something like:

(defpycallback ("python_name" :module py-mod) return-type
    (args...)
  ...)
(defpycallback (("python_name" lisp-name) :module py-mod) return-type
    (args...)
  ...)

Though the keyword arguments do have the advantage of being very obvious at a glance.

Tests

The callback / module tests are kind of...insane? Why do you use *callback-ret*, instead of simply returning a tuple from the callback function? Why the copy-pasted macroexpansion from your .SYNTAX package, rather than something more straightforward, like:

(lift:ensure (equalp (run "callbacks_test.args(22, \"a\")")
                     '(nil (22 "a") 22)))

Also, I'm not sure I understand the difference between the callbacks.lisp tests and the modules.lisp tests. They appear to be nearly identical.

Misc.

PYTHON package nickname

Okay, okay, I yield. You can have that. :)

Symbol Comparison

#'equalp is serious overkill for symbol comparisons. (eq var 'symbol) is sufficient. (I'm not even sure why I used #'eql, since that's more than necessary.)

Pathnames

Would it make sense to define a method on CFFI:TRANSLATE-TO-FOREIGN for converting pathnames to CFFI:FOREIGN-STRING-TYPE? (Well, or possibly our own subclass of f-s-t, so we don't conflict with anybody else.) Then, for example, fopen could be called as (fopen #p"foo") rather than (fopen (princ-to-string #p"foo")). Though maybe that doesn't matter, since fopen is meant to be internal anyway.

Along the lines of passing a pathname to fopen / other functions, would it make sense to merge that pathname with *DEFAULT-PATHNAME-DEFAULTS* so we're always giving the python interpreter an absolute path? I'm a little worried that if we don't fully-resolve the pathname beforehand people will end up in a situation where *d-p-d* and whatever cpython uses to resolve relative names (current directory?) diverge and end up with something like the following happening:

(with-open-file (f #p"foo") ;; opens "/path/to/foo"
  (python:run #p"foo") ;; runs "/another/path/to/foo"
  ...)

which would be rather confusing. But we probably can't completely prevent that anyway, so maybe it'd be best just to document it and let users figure it out?

fopen / fclose

Since fopen can fail, it might be a good idea to check the return value and error out if it returns a null pointer.

Would it make sense for fopen's mode argument to accept a keyword value (say, matching the ones to #'CL:OPEN) and error for invalid ones? E.g., (fopen #p"file" :input). Though, since it's only there for b-b's internal use, that's probably overkill.

Are you sure fclose() returns a pointer? man fclose says it returns an int on my system. Also, since fclose() can error, do we need to check its return value? (Would there be any point?)

Python 3.2 and up have a _Py_fopen wrapper around fopen. Would it make sense to use that instead of straight fopen, if available? (I doubt much else in burgled-batteries works past 2.7, so I'm not worried about it, just throwing it out there.)

Rename module.lisp to modules.lisp

Just for consistency with all the other plural file names.

DEFPYMODULE / IN-PYMODULE

Nice! It might be better if their expansions included an eval-when form. Something like:

(eval-when (:compile-toplevel :load-toplevel :execute)
  (%def/in-pymodule ...))

I've run into issues before when I don't do that, but eval-when kind of confuses me so I'm never quite sure when it is and isn't necessary.

Nice Work

Okay, I think that's it. Like I said, it's a really great start. I look forwarding to merging it in as we get it all hammered down.

Edit: Fix formatting. Apparently github doesn't apply markdown when replying via e-mail? Edit2: Well, I tried to fix the formatting. It looks good when I hit "Preview", but "Preview" and reality apparently disagree.

mmontone commented 9 years ago

On 24/06/15 19:40, pinterface wrote:

Okay, proper review time! My apologies for failing to meet my own deadline.

No problem. Thank you for going through my code.

Overall: This is a fantastic start! So much good stuff here!

This response is long, but mostly because it contains a lot of me thinking out loud (you asked for a discussion!), so if you think I'm wrong somewhere, do feel free to argue. :)

Functions Dealing with FILE* (fopen, PyRun_AnyFile, etc.)

These should probably use a new FILE-POINTER type or similar (maybe even just call it FILE*), rather than a raw :POINTER type. Firstly so it's more self-documenting (:pointer is unhelpfully vague), and secondly for future expansion.

E.g., so in the future we have the option of enabling something like:

(with-open-file (f path)
(run.any-file f path))

Which seems kind of farcical to me, but it's what the cpython guys built for their API.

Actually, I just needed the file api and so I did what I could to enable it. Probably not optimal, since I'm not very well acquainted with the FFI stuff. So, let's do what you suggest.

DEFPYCALLBACK

It might be a good idea for us to figure out what exactly the low-level and high-level DEFPYCALLBACK syntaxes should be, particularly around argument specification. I've thrown some ideas in below, but you're actually writing callbacks so I'd definitely like to hear your thoughts / ideas.

Yes. There's definitely room for discussion and ideas regarding the definition of callbacks, specially as taste is involved here.

Low-level (PYTHON.CFFI package)

I'd recommend throwing an error if &optional or &rest are encountered, since neither are presently supported.

Agreed.

Positional and &key arguments should probably be bound regardless of whether return-type is :pointer. It seems silly to make people deal with the pain of argument unpacking just because they're also willing to deal with pointers.

Ok. I tried to follow the low level callbacks design that was already there in your original implementation. I think the decision to deal with pointers or not depended on whether the return type was :pointer or not, if I understood correctly.

Syntax Ideas

Take strings with python names instead of symbols by default, much like the low-level DEFPYFUN. Something like:

(defpycallback name return-type ("arg1" ... &key "keyword_arg" ...)
;; here arg1 and keyword-arg are bound as variables
)

With, of course, the ability to specify the lisp name, if desired.

Possibly by just replacing the keyword parameter specifier in an ordinary lambda list[1] and replacing it with a string for the python name:

(defpycallback name return-type
(;; required parameter with separate python and lisp names
("arg1" arg-1)
;; key arg with python name, lisp name, default value, and
;; supplied-p variable
&key (("keyword_arg" key-arg) 'default key-arg-supplied?))
;; here arg-1, key-arg, and key-arg-supplied? are bound as variables
)

[1] http://clhs.lisp.se/Body/03_da.htm

Ok, but why not symbols as default, that can be converted to a python string ('foo-bar -> "foo_bar"), and a string optionally if the user wants to be more precise? Is this a matter of taste only?

Aaand I left the types out of that syntax idea, so obviously that needs some refinement. CL's keyword specifications are surprisingly complicated--I'm very open to ideas on how to represent the full range of possibilities, plus deal with CFFI types, in a way that isn't incredibly painful for people trying to write callbacks. (I'm definitely not asking you to make supplied-p parameters and default values and all that jazz work, I'd just like to have the syntax nailed down so if you or me or somebody else tries to implement those later we haven't painted ourselves into a corner.)

Ok. Not so sure what to do about that at the moment that I'm writing this.

High-level (PYTHON package)

The default return-type should probably not be :pointer. After all, the point of the high-level API is to deal with pointers for you as much as possible. Not sure if we could reasonably default it to OBJECT, or if it would make sense to require the user always specify the return-type.

(... :python-name "foo" :return-type bar) feels a bit verbose. Would it make any sense to compact it down a wee bit? Maybe something like:

(defpycallback ("python_name" :module py-mod) return-type
(args...)
...)
(defpycallback (("python_name" lisp-name) :module py-mod) return-type
(args...)
...)

I have no problem with that, although I'm not sure either if require the user to always specify the return-type is necessary.

This is probably something I don't understand, but I think if you have :pointer as callback type, then it is converted to the appropriate Python object on the Python side, which is what matters for callbacks? (at least I had no problems in the callbacks demo).

Though the keyword arguments do have the advantage of being very obvious at a glance.

Tests

The callback / module tests are kind of...insane?

Probably :)

Why do you use

*callback-ret*, instead of simply returning a tuple from the callback function?

I was not being able to get the callback return object for some reason. But yes, that's just wrong.

Why the copy-pasted macroexpansion from your .SYNTAX package,

rather than something more straightforward, like:

(lift:ensure (equalp (run "callbacks_test.args(22, \"a\")")
'(nil (22 "a") 22)))

Nothing to say here. You are right. I will change that.

Also, I'm not sure I understand the difference between the callbacks.lisp tests and the modules.lisp tests. They appear to be nearly identical.

The difference is I tried to test the low level callbacks api that you had implemented in callbacks.lisp, and the high-level callbacks I implemented and how they behave with modules and their construction in modules.lisp.

Misc.

PYTHON package nickname

Okay, okay, I yield. You can have that. :)

Cooooool! ;-)

Symbol Comparison

#'equalp is serious overkill for symbol comparisons. (eq var 'symbol) is sufficient. (I'm not even sure why I used #'eql, since that's more than necessary.)

Noted

Pathnames

Would it make sense to define a method on CFFI:TRANSLATE-TO-FOREIGN for converting pathnames to CFFI:FOREIGN-STRING-TYPE? (Well, or possibly our own subclass of f-s-t, so we don't conflict with anybody else.) Then, for example, fopen could be called as (fopen #p"foo") rather than (fopen (princ-to-string #p"foo")). Though maybe that doesn't matter, since fopen is meant to be internal anyway.

Along the lines of passing a pathname to fopen / other functions, would it make sense to merge that pathname with *DEFAULT-PATHNAME-DEFAULTS* so we're always giving the python interpreter an absolute path? I'm a little worried that if we don't fully-resolve the pathname beforehand people will end up in a situation where d-p-d and whatever cpython uses to resolve relative names (current directory?) diverge and end up with something like the following happening:

(with-open-file (f #p"foo") ;; opens "/path/to/foo"
(python:run #p"foo") ;; runs "/another/path/to/foo"
...)

which would be rather confusing. But we probably can't completely prevent that anyway, so maybe it'd be best just to document it and let users figure it out?

fopen / fclose

Since fopen can fail, it might be a good idea to check the return value and error out if it returns a null pointer.

Would it make sense for fopen's mode argument to accept a keyword value (say, matching the ones to #'CL:OPEN) and error for invalid ones? E.g., (fopen #p"file" :input). Though, since it's only there for b-b's internal use, that's probably overkill.

Are you sure fclose() returns a pointer? man fclose says it returns an int on my system. Also, since fclose() can error, do we need to check its return value? (Would there be any point?)

Python 3.2 and up have a _Py_fopen wrapper around fopen. Would it make sense to use that instead of straight fopen, if available? (I doubt much else in burgled-batteries works past 2.7, so I'm not worried about it, just throwing it out there.)

I will need more time to give you some feedback on this pathnames stuff. Maybe this is something you can take care of? You know more about this than me.

Rename module.lisp to modules.lisp

Just for consistency with all the other plural file names.

No problem.

DEFPYMODULE / IN-PYMODULE

Nice! It might be better if their expansions included an eval-when form. Something like:

(eval-when (:compile-toplevel :load-toplevel :execute)
(%def/in-pymodule ...))

I've run into issues before when I don't do that, but eval-when kind of confuses me so I'm never quite sure when it is and isn't necessary.

No problem.

Nice Work

Okay, I think that's it. Like I said, it's a really great start. I look forwarding to merging it in as we get it all hammered down.

Perfect. I will try to come back to you with some code when I have time to work on these things.

Thank you

pinterface commented 9 years ago

On 2015.06.24 22:52, Mariano Montone wrote:

On 24/06/15 19:40, pinterface wrote:

Functions Dealing with FILE* (fopen, PyRun_AnyFile, etc.)

...

Actually, I just needed the file api and so I did what I could to enable it. Probably not optimal, since I'm not very well acquainted with the FFI stuff. So, let's do what you suggest.

That's fine. I can start from your work and expand it. :) Actually, I should have mentioned that: I'm totally gonna fire up a branch and play with some of these ideas over the next couple of weeks. It's not all on you! You've inspired me to dig in and hammer out some of this stuff.

DEFPYCALLBACK

Low-level (PYTHON.CFFI package)

Positional and &key arguments should probably be bound regardless of whether return-type is :pointer. It seems silly to make people deal with the pain of argument unpacking just because they're also willing to deal with pointers.

Ok. I tried to follow the low level callbacks design that was already there in your original implementation. I think the decision to deal with pointers or not depended on whether the return type was :pointer or not, if I understood correctly.

Hrm... I guess I should mention: the callback stuff I had was nowhere near a fleshed out API design. It was just some experimentation to get a feel for things. Don't feel like there's anything there you have to stick to, if you think you've got a better way of doing it!

I'm not saying the low-level stuff should deal with type translation of arguments for return-type :pointer. I'm suggesting that if the user specifies a callback with a key "foo", then within the body of the callback, the variable foo should already be bound to the value of the dict key "foo" regardless (under :pointer, foo would be a pointer to a Python object; under a translated type it would be a lisp object).

Which is probably about as clear as my first explanation. I'll play around with implementing it. Sometimes that's the easiest way to explain something. :)

Syntax Ideas

Take strings with python names instead of symbols by default, much like the low-level DEFPYFUN. Something like:

(defpycallback name return-type ("arg1" ... &key "keyword_arg" ...)
;; here arg1 and keyword-arg are bound as variables
)

With, of course, the ability to specify the lisp name, if desired.

...

Ok, but why not symbols as default, that can be converted to a python string ('foo-bar -> "foo_bar"), and a string optionally if the user wants to be more precise? Is this a matter of taste only?

I was suggesting that way because that's how the low-level DEFPYFUN macro works, but you're absolutely right that it doesn't actually matter. I believe the expression is "Six of one, half a dozen of the other.".

In fact, I've just realized that CFFI works either way. If you give it a string, it uses the string for the C side and makes itself a symbol. If you give it a symbol, it uses that for the lisp side and makes itself a C name. If you give it both, it uses both as appropriate. Man, that's the best way. Then it works regardless. So that's probably the way to move forward--just work with whatever the user gives us and generate the thing they don't give us.

(Which means I need to go fix some existing code to work in either direction. Man, why didn't I notice that originally?)

Aaand I left the types out of that syntax idea, so obviously that needs some refinement. CL's keyword specifications are surprisingly complicated--I'm very open to ideas on how to represent the full range of possibilities, plus deal with CFFI types, in a way that isn't incredibly painful for people trying to write callbacks. (I'm definitely not asking you to make supplied-p parameters and default values and all that jazz work, I'd just like to have the syntax nailed down so if you or me or somebody else tries to implement those later we haven't painted ourselves into a corner.)

Ok. Not so sure what to do about that at the moment that I'm writing this.

That's fine! It'll take some thought, and I'll be thinking about it. If I come up with something, I'll probably try to toss together an implementation so you can try it and see how you like it, and we can refine from there. On a related note: How are you liking the syntax you came up with so far? Not too terrible?

High-level (PYTHON package)

The default return-type should probably not be :pointer. After all, the point of the high-level API is to deal with pointers for you as much as possible. Not sure if we could reasonably default it to OBJECT, or if it would make sense to require the user always specify the return-type.

...

I have no problem with that, although I'm not sure either if require the user to always specify the return-type is necessary.

This is probably something I don't understand, but I think if you have :pointer as callback type, then it is converted to the appropriate Python object on the Python side, which is what matters for callbacks? (at least I had no problems in the callbacks demo).

Right, the issue isn't when returning a :pointer, since that's the lisp side dealing directly with the Python object. The trouble is when the lisp side returns a lisp object that has to be converted into a Python object. E.g., is nil a zero-length tuple or boolean false? Is an array like #(104 101 108 108 111) an array of ints or a bytestring?

Tests

The callback / module tests are kind of...insane?

Probably :)

Why do you use

*callback-ret*, instead of simply returning a tuple from the callback function?

I was not being able to get the callback return object for some reason. But yes, that's just wrong.

Hrm... that may very well be a bug on my end! Like I mentioned above, the callbacks were just a quick experiment, not a fleshed out thing. I'll play around with that and see if I can get it to work.

Also, I'm not sure I understand the difference between the callbacks.lisp tests and the modules.lisp tests. They appear to be nearly identical.

The difference is I tried to test the low level callbacks api that you had implemented in callbacks.lisp, and the high-level callbacks I implemented and how they behave with modules and their construction in modules.lisp.

Oh! Right! Man, what was I on when I decided naming the low-level and high-level macros the same thing and putting them in different packages was a good idea? I even confuse myself with that! Bleh.

Pathnames

...

fopen / fclose

...

I will need more time to give you some feedback on this pathnames stuff. Maybe this is something you can take care of? You know more about this than me.

Yeah, no problem. I don't expect you to write any of that! Just thinking out loud.

Nice Work

Okay, I think that's it. Like I said, it's a really great start. I look forwarding to merging it in as we get it all hammered down.

Perfect. I will try to come back to you with some code when I have time to work on these things.

Excellent. I'll work on some of it, too. Particularly the "Ooooh, neat, feature pile on!" parts. :)

mmontone commented 9 years ago

On 25/06/15 02:50, pinterface wrote:

On 2015.06.24 22:52, Mariano Montone wrote:

On 24/06/15 19:40, pinterface wrote:

Functions Dealing with FILE* (fopen, PyRun_AnyFile, etc.)

...

Actually, I just needed the file api and so I did what I could to enable it. Probably not optimal, since I'm not very well acquainted with the FFI stuff. So, let's do what you suggest.

That's fine. I can start from your work and expand it. :) Actually, I should have mentioned that: I'm totally gonna fire up a branch and play with some of these ideas over the next couple of weeks. It's not all on you! You've inspired me to dig in and hammer out some of this stuff.

DEFPYCALLBACK

Low-level (PYTHON.CFFI package)

Positional and &key arguments should probably be bound regardless of whether return-type is :pointer. It seems silly to make people deal with the pain of argument unpacking just because they're also willing to deal with pointers.

Ok. I tried to follow the low level callbacks design that was already there in your original implementation. I think the decision to deal with pointers or not depended on whether the return type was :pointer or not, if I understood correctly.

Hrm... I guess I should mention: the callback stuff I had was nowhere near a fleshed out API design. It was just some experimentation to get a feel for things. Don't feel like there's anything there you have to stick to, if you think you've got a better way of doing it!

I'm not saying the low-level stuff should deal with type translation of arguments for return-type :pointer. I'm suggesting that if the user specifies a callback with a key "foo", then within the body of the callback, the variable foo should already be bound to the value of the dict key "foo" regardless (under :pointer, foo would be a pointer to a Python object; under a translated type it would be a lisp object).

Oh. I understand. Agree.

Which is probably about as clear as my first explanation. I'll play around with implementing it. Sometimes that's the easiest way to explain something. :)

Syntax Ideas

Take strings with python names instead of symbols by default, much like the low-level DEFPYFUN. Something like:

(defpycallback name return-type ("arg1" ... &key "keyword_arg" ...)
;; here arg1 and keyword-arg are bound as variables
)

With, of course, the ability to specify the lisp name, if desired.

...

Ok, but why not symbols as default, that can be converted to a python string ('foo-bar -> "foo_bar"), and a string optionally if the user wants to be more precise? Is this a matter of taste only?

I was suggesting that way because that's how the low-level DEFPYFUN macro works, but you're absolutely right that it doesn't actually matter. I believe the expression is "Six of one, half a dozen of the other.".

In fact, I've just realized that CFFI works either way. If you give it a string, it uses the string for the C side and makes itself a symbol. If you give it a symbol, it uses that for the lisp side and makes itself a C name. If you give it both, it uses both as appropriate. Man, that's the best way. Then it works regardless. So that's probably the way to move forward--just work with whatever the user gives us and generate the thing they don't give us.

(Which means I need to go fix some existing code to work in either direction. Man, why didn't I notice that originally?)

That's good news from my perspective. My problem is that I don't like strings in general, and try to avoid them when possible (use symbols instead). But yeah, it is not a technical objection I have, but question of taste. (A mental problem I have probably).

You say symbols are converted to strings.. Does it make sense to convert them with something like the python-string function in module.lisp? Maybe that would be even better, I don't know...

Aaand I left the types out of that syntax idea, so obviously that needs some refinement. CL's keyword specifications are surprisingly complicated--I'm very open to ideas on how to represent the full range of possibilities, plus deal with CFFI types, in a way that isn't incredibly painful for people trying to write callbacks. (I'm definitely not asking you to make supplied-p parameters and default values and all that jazz work, I'd just like to have the syntax nailed down so if you or me or somebody else tries to implement those later we haven't painted ourselves into a corner.)

Ok. Not so sure what to do about that at the moment that I'm writing this.

That's fine! It'll take some thought, and I'll be thinking about it. If I come up with something, I'll probably try to toss together an implementation so you can try it and see how you like it, and we can refine from there. On a related note: How are you liking the syntax you came up with so far? Not too terrible?

You mean the syntax of the high-level defpycallback? It is ok for me...

High-level (PYTHON package)

The default return-type should probably not be :pointer. After all, the point of the high-level API is to deal with pointers for you as much as possible. Not sure if we could reasonably default it to OBJECT, or if it would make sense to require the user always specify the return-type.

...

I have no problem with that, although I'm not sure either if require the user to always specify the return-type is necessary.

This is probably something I don't understand, but I think if you have :pointer as callback type, then it is converted to the appropriate Python object on the Python side, which is what matters for callbacks? (at least I had no problems in the callbacks demo).

Right, the issue isn't when returning a :pointer, since that's the lisp side dealing directly with the Python object. The trouble is when the lisp side returns a lisp object that has to be converted into a Python object. E.g., is nil a zero-length tuple or boolean false? Is an array like #(104 101 108 108 111) an array of ints or a bytestring?

Ok.

Tests

The callback / module tests are kind of...insane?

Probably :)

Why do you use

*callback-ret*, instead of simply returning a tuple from the callback function?

I was not being able to get the callback return object for some reason. But yes, that's just wrong.

Hrm... that may very well be a bug on my end! Like I mentioned above, the callbacks were just a quick experiment, not a fleshed out thing. I'll play around with that and see if I can get it to work.

Oh. Ok, I will try to play a bit with it also.

Also, I'm not sure I understand the difference between the callbacks.lisp tests and the modules.lisp tests. They appear to be nearly identical.

The difference is I tried to test the low level callbacks api that you had implemented in callbacks.lisp, and the high-level callbacks I implemented and how they behave with modules and their construction in modules.lisp.

Oh! Right! Man, what was I on when I decided naming the low-level and high-level macros the same thing and putting them in different packages was a good idea? I even confuse myself with that! Bleh.

:)

Pathnames

...

fopen / fclose

...

I will need more time to give you some feedback on this pathnames stuff. Maybe this is something you can take care of? You know more about this than me.

Yeah, no problem. I don't expect you to write any of that! Just thinking out loud.

Nice Work

Okay, I think that's it. Like I said, it's a really great start. I look forwarding to merging it in as we get it all hammered down.

Perfect. I will try to come back to you with some code when I have time to work on these things.

Excellent. I'll work on some of it, too. Particularly the "Ooooh, neat, feature pile on!" parts. :)

— Reply to this email directly or view it on GitHub https://github.com/pinterface/burgled-batteries/pull/6#issuecomment-115114160.