Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature suggestion: supporting attribute calls with argument given in map #46

Closed
davidpham87 opened this issue Jan 18, 2020 · 7 comments · Fixed by #57
Closed

feature suggestion: supporting attribute calls with argument given in map #46

davidpham87 opened this issue Jan 18, 2020 · 7 comments · Fixed by #57

Comments

@davidpham87
Copy link
Contributor

Hello,

Thanks a lot for the library, it has been a real pleasure to use.

For now, we can call a function like this:

(py/$a np linspace :start 0 :stop_button:  1)

but it would be great if we could integrate a macro, say $a** such that we could perform

(py/$a** np linspace {:start 0 :stop 1})

Obviously we could also do this

(defn $a** [x attr m] (py/call-attr-kw x attr nil m))

However, I think it would be a beneficial addition to the library.

Best regards,
David

@jjtolton
Copy link
Contributor

@davidpham87 sorry for delayed response.

How would you feel about this:

(py. obj **method arg1 arg2 arg3 ... argN kwargs) where last argument is interpreted as a map?

That same modification would then percolate to

(py.. obj (**method1 arg1 arg2 ... kwargs) (**method2 arg1 arg2 ... kwargs))

or would you prefer

(py. obj **method args kwargs)?

I could probably also do (py. obj method *args **kwargs) pretty easily with a little macro magic but I'd like to hear how @cnuernber feels about that.

@jjtolton
Copy link
Contributor

jjtolton commented Jan 20, 2020

The last option would be the most flexible.

(py. obj method *args) would be equivalent to (py. obj method arg1 arg2 arg3 ... argN) and this could be mixed/matched with
(py. obj method **kwargs) which would be equivalent to (py. obj method :attr1 val1 :att2 :val2 ... :attrN :valN)

@davidpham87
Copy link
Contributor Author

@jjtolton No problem, you have been helping me on Zulip (I am Neo). I don't know which dark magic you use in macro world, but this would be great

(py. obj method **kwargs}) ;; or (py. module fn **{:as kwargs})

but I think

(py.** obj method kwargs)  ;;  

would be the cleanest of all. It could be just a function wrapping py/call-attr-kw.

But it would be great with module/function calls like

(np/linspace **{:start 0 :stop 1})

Let's wait on @cnuernber for his opinion.

@jjtolton
Copy link
Contributor

jjtolton commented Jan 21, 2020

Alright after thinking it over, (py. obj method *args **kwargs) LITERAL syntax has a few problems. For instance, *args and **kwargs are both valid symbol names, so then I'd need to provide an escape character -- and now things are already getting pretty messy and a bit contraventional to Clojure idioms.

I'll investigate a (py.** method kwargs) macro and corresponding (py.. obj (**method kwargs)) form.

@jjtolton
Copy link
Contributor

@davidpham87 please review #57 and let me know if that looks like it will work the way you were expecting :)

@davidpham87
Copy link
Contributor Author

Thanks a lot for the work. I really love the syntax. I think though we should not support the multiarities of theses methods as it is counterintuitive. Maybe have an alias for py/call-kw to have the two arities.

The reason it bothers me is the **method variant as change the position of the kwargs (I know it is common in Clojure, but for a specialized library, what do you think of going with the “least amount of surprise” path?

@jjtolton
Copy link
Contributor

The kwargs is always last in **, which means if you use it with just one arg, that arg will be the kwargs, which would cover your original use-case. I figure that is the arrity that will probably be used most, but now there is an arrity that covers all options. But I’ll ask for some more feedback and see what the consensus is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants