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

variable numbers of keyword arguments as dict #4916

Closed
WestleyArgentum opened this issue Nov 25, 2013 · 18 comments
Closed

variable numbers of keyword arguments as dict #4916

WestleyArgentum opened this issue Nov 25, 2013 · 18 comments
Assignees
Labels
breaking This change will break code keyword arguments f(x; keyword=arguments)
Milestone

Comments

@WestleyArgentum
Copy link
Member

foo(; args...)

Inside of a function with this signature, args will be an array of tuples containing symbols and values. Cool, but the array will be unordered, so I pretty much have to loop over it trying to match symbols.

I think it would be really cool if args came in as a dictionary. I'm guessing there would be some overhead involved in that, and that's perhaps why it has not been done. But I wonder if the overhead involved in looping over symbols and doing a bunch of string comparisons (I think that's what's happening, could be wrong) might even outweigh the dict creation.

I didn't see an issue, but has this been considered?

@JeffBezanson
Copy link
Member

I'd recommend doing some performance experiments. I suspect a dict wouldn't be a win until the number of keyword arguments was pretty large (probably > 20), which doesn't happen very often.

@kmsquire
Copy link
Member

One option might be to make a new dictionary type for small dictionaries
like this, which could implement a dictionary interface, but simply be
stored as two lists or a list of tuples.

On Sunday, November 24, 2013, Jeff Bezanson wrote:

I'd recommend doing some performance experiments. I suspect a dict
wouldn't be a win until the number of keyword arguments was pretty large
(probably > 20), which doesn't happen very often.


Reply to this email directly or view it on GitHubhttps://github.com//issues/4916#issuecomment-29172755
.

@WestleyArgentum
Copy link
Member Author

I'll try to do some tests soon. I like @kmsquire's idea, at least then use code isn't cluttered with blocks of if elseif elseif... statements

@ghost ghost assigned JeffBezanson Nov 26, 2013
@remusao
Copy link
Contributor

remusao commented Mar 4, 2014

Don't know if @WestleyArgentum had time to dig into this issue, but I think it would be a good idea to have varargs keyword arguments as a Dict (like in Python for example). From usability point of view I think it would be a win (I find it more natural).

If there is a performance cost, as @kmsquire said, maybe we could try to implement a Dict type efficient for small amount of values inside ?

Edit:
I tried to do some test here
https://gist.github.com/remusao/9344557

I compare access time to Vector{(Symbol, ASCIIString)} and Dict{Symbol, ASCIIString} with various number of elements (1 -> 100). For each test case, I look 100k for every element in the collection.

On my machine:

  • i7 860 (2.8 Ghz)

It seems that Array are faster until number of elements reach 14 (I agree that it is a lot of keyword arguments...). And then Dict becomes faster. But while Array is faster for few elements, the difference seems quite reduced. For 100k x (access to each element of the collection), the difference is always x millisecs.

Tell me if I'm wrong about the bench, but from a performance point of view, the advantage of Array over Dict seems negligible.

@stevengj
Copy link
Member

Note that the issue of having a specialized Associative type for small dictionaries, perhaps even specialized to dictionaries with Symbol keys, was also raised in #7959.

@ntessore
Copy link

I think such a situation could be handled in a uniform way with a function that goes over pairs:

julia> function findbykey(a, key)
           for (k, v) in a
               k == key && return v
           end
           error("not found")
       end
findbykey (generic function with 1 method)

julia> d = { "a" => "A", "b" => "B", "c" => "C" }
Dict{Any,Any} with 3 entries:
  "c" => "C"
  "b" => "B"
  "a" => "A"

julia> findbykey(d, "c")
"C"

julia> d = [ ("a", "A"), ("b", "B"), ("c", "C") ]
3-element Array{(ASCIIString,ASCIIString),1}:
 ("a","A")
 ("b","B")
 ("c","C")

julia> findbykey(d, "c")
"C"

@stevengj
Copy link
Member

@ntessore, except that this is a terrible way to look up something in a Dict (though I suppose you could define findbykey(a::Associative, key) = a[key]). You really want to overload get, and that requires a new type.

The point is that there are potentially several places where a specialized SmallDict type could be useful, and if we were to define such a type it would be nice to use it for keywords too.

@ntessore
Copy link

Of course it does not make any sense for a Dict, but that was the first container holding pairs I could think of. My point was that it is generally useful to extract something from a set of (key, value) pairs, so maybe there is room for a generic function (that one can then specialise as necessary, e.g. for dictionaries).

@stevengj
Copy link
Member

(In Scheme, the findbykey function is called assoc.)

@remusao
Copy link
Contributor

remusao commented Nov 11, 2014

Why not adding a method get to array of pairs?
On Nov 11, 2014 8:40 PM, "Steven G. Johnson" notifications@github.com
wrote:

(In Scheme, the findbykey function is called assoc.)

Reply to this email directly or view it on GitHub
#4916 (comment).

@stevengj
Copy link
Member

@remusao, because arrays already have a get method: get(A, i, default) or A[i] look up by index.

@GlenHertz
Copy link
Contributor

I sometimes want to print out the args in the same order so I wouldn't want to have this turned into an unordered dict. I would much rather have a new datatype that was optimized for a list of pairs that was really fast for <5 pairs.

@StefanKarpinski
Copy link
Member

Varargs keywords will either be immutable dicts or named tuples, so this will be fixed in 1.0 one way or another.

@JeffBezanson JeffBezanson added the keyword arguments f(x; keyword=arguments) label Jul 20, 2017
@micklat
Copy link
Contributor

micklat commented Jul 24, 2017

Whatever you do, please preserve the argument order (so either a tuple or an ordered dictionary would keep me happy).

@ViralBShah
Copy link
Member

Would be great to have an update on if this is happening by feature freeze. A PR exists in #21915, but that is not marked 1.0.

@ViralBShah ViralBShah added the triage This should be discussed on a triage call label Nov 19, 2017
@JeffBezanson
Copy link
Member

I'll update that PR to use named tuples.

@oscardssmith
Copy link
Member

Doesn't that mean that every time you use a function with a different ordering of keyword arguments it will have to create a new specialization of Tupple? Is there any way around this?

@JeffBezanson
Copy link
Member

Basically yes, but that won't necessarily be a problem in practice. We'll find out.

@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Nov 19, 2017
JeffBezanson added a commit that referenced this issue Nov 26, 2017
JeffBezanson added a commit that referenced this issue Nov 26, 2017
JeffBezanson added a commit that referenced this issue Nov 26, 2017
JeffBezanson added a commit that referenced this issue Nov 27, 2017
JeffBezanson added a commit that referenced this issue Nov 27, 2017
ararslan pushed a commit that referenced this issue Dec 2, 2017
JeffBezanson added a commit that referenced this issue Dec 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code keyword arguments f(x; keyword=arguments)
Projects
None yet
Development

No branches or pull requests