use keyword arguments

[JeffBezanson]

Now that they exist they should be used instead of dictionaries as appropriate.

[timholy]

I agree. @jverzani, what are your views on the path forward here? And how bad do you think it will be?

[timholy]

(My personal suspicion is that updating the documentation & blog will be at least as bad as the code itself, but I could be wrong.)

[jverzani]

Definitely the way to go. I'll have a crack at it sometime soon.

[jverzani]

Okay, I pushed a branch kwargs that should do this. One thing that I don't know and had to hack around was how to pass keyword args from one function down to another:

That is if

g(; a= 1) = println(a)

Then how do I define f to pass things along. I tried:

f(;kwargs...) = g(kwargs...) # and
f(;kwargs...) = g(kwargs)

but neither worked to pass f(a=2)

[simonster]

f(;kwargs...) = g(;kwargs...)

although the performance tips note:

Passing dynamic lists of named arguments, as in f(x; names...), can be slow and should be avoided in performance-sensitive code.

[JeffBezanson]

It should be fine for GUI code.

[jverzani]

Okay, thanks for the pointer. Twice this week I should have rtfm'ed. Live and learn. Checked in the changes into the kwargs branch.

@timholy suggested changing the names of tk_configure, tk_bind, ... to just configure, bind, .... The only real conflict is with tk_bind, and there it isn't such an issue except that bind in the Tk sense isn't so related to bind in the julia sense. If this is a good idea, now would be the time to do it. Any thoughts?

[timholy]

Thanks for doing this!

I'll let Jeff comment on the name changes. I don't have strong feelings one way or another.

Along the lines of API bikeshedding, this is a bit awkward:

julia> win = Toplevel("Hi")
Tk widget of type Tk_Toplevel

julia> width(win)
ERROR: no method width(Tk_Toplevel,)

julia> width(win.w)
200

julia> get_width(win)
200

Thoughts? The symmetry between get_width and set_width is nice, but it conflicts with Base.Graphics etc.

[jverzani]

Okay changed those to be consistent. I'm reluctant to merge this all into master as it breaks some things in Winston and ImageView (I didn't keep a graceful fallback when specifying values with a dict). The needed changes to those two are modest, but need to be coordinated.

[timholy]

I didn't mean to imply, however, that I was full of confidence that changing get_width to width is the right answer; again the getindex and setindex! is an example of the other approach. It will be interesting to see what others say.

[jverzani]

Seems appropriate. The Tk command is called width, so it fits there. As well, in Qt they would have methods width and setWidth. Gtk would be different. Maybe of more interest is what does width actually refer to? As it is now it is the width as drawn, which seems most natural. The size request can be found from widget[:width]. The request is set with set_width or widget[:width] = width.

On Wed, May 22, 2013 at 1:10 PM, Tim Holy notifications@github.com wrote:

I didn't mean to imply, however, that I was full of confidence that changing get_width to width is the right answer; again the getindex and setindex! is an example of the other approach. It will be interesting to see what others say.

— Reply to this email directly or view it on GitHubhttps://github.com/JuliaLang/Tk.jl/issues/25#issuecomment-18293086 .

John Verzani Chair, Department of Mathematics College of Staten Island, CUNY verzani@math.csi.cuny.edu

[timholy]

OK.

Regarding the kwargs branch, I'm not at all worried about breakage to ImageView. I have a "get data to someone" deadline, but once I get done with that I'll make the changes to Winston and submit a pull request.

I also checked, and it looks like there aren't any other packages besides Winston and my own that list Tk as a requirement. So I think these changes will be fairly minimal in their impact.

[JeffBezanson]

I haven't thought a lot about how to handle properties in general. width and height are important enough that it makes sense to have generic functions for them, so those should be used when possible. We probably don't want getX and setX methods for every possible property though. A dictionary-style interface like you have seems totally reasonable.

I think I would always want the drawn width, not the requested width. But in any case, since they are different properties they should have different names.

Winston's Table is an example of something that has both indexing (getindex and setindex!) and properties (via Winston's getattr). So in the long run those may need to be separate interfaces. I have started planning overloading of a.b and a.b = c, and that might be the best thing to use. It will work as follows:

getfield(w::Widget, ::Field{:width}) = ...

so the properties are each separate methods but have "names", which has many advantages.

[jverzani]

Following Jeff's comment, I thought about this and decided set_width, set_height and set_size only really work with setting a window geometry, as that is what width and height return. So those methods are now only defined for toplevel windows. For other size management, the user can drop down and set the widget's -width or -height option, if they exist, using the indexing interface, e.g [:width], which is similar to Python's Tk bindings.

@timholy Tim, do you want to merge these changes into the master, as that will break your code and a bit in Winston?

[timholy]

That sounds good. My plan is:

1. Write a bunch of Winston tests (there's hardly anything there currently), submit pull request
2. Convert Winston to the Tk kwargs branch, submit pull request
3. If & when those are merged, hit merge on your branch
4. Fix ImageView

[timholy]

Hmm, 1-3 were much easier than expected. I hadn't noticed Winston's examples/ directory, that made it rather easy to increase the number of tests.

I'm done for tonight, but given Jeff's work hours perhaps the kwargs branch will be merged by morning.

[JeffBezanson]

Ok, let's go ahead and merge.