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

Mapping empty Arrays #10307

Closed
MillCheck opened this issue Feb 24, 2015 · 11 comments
Closed

Mapping empty Arrays #10307

MillCheck opened this issue Feb 24, 2015 · 11 comments

Comments

@MillCheck
Copy link

typeof(float(String[])) is Array{String,1}. This can cause performance issues.
typeof(map(float, String[])) behaves the same.

Even worse, runtime errors if someone reads numbers from a line and later tries to push! something to them - if the line was empty, you get an ERROR.

@simonster
Copy link
Member

I think this is already covered by #210/#1864/ #10269. Without knowing what type the function returns, we can't get the proper return type for an empty array. But even if we could do that, type inference would also have to know about the return type to solve the performance issue. Parametric function types should solve all of this.

@johnmyleswhite
Copy link
Member

I believe not all functions will be parametric and hence map of empty arrays will still do odd things sometimes.

@JeffBezanson
Copy link
Member

For those particular conversion functions, e.g. float(), it would be totally reasonable for them to pick the result type and use map! instead.

@MillCheck
Copy link
Author

I think making it return Any[] would be reasonable then

@JeffBezanson
Copy link
Member

Array{Union()} is an even better general result for an empty array.

@StefanKarpinski
Copy link
Member

I feel like we've already been here: [] used to construct a Vector{Union()} (it was called Vector{None} back then). Then we decided that was a pretty useless type, so we made it do Any[] instead. What makes this situation different, I suppose is type stability. You don't want the return type signature of map(x->x+1, Vector{Int}) to be Union(Vector{Int},Vector{Any}), and Union(Vector{Int},Vector{Union()}) is a bit better since you know that any element taken out of something of that type must be an Int, but it's still kind of an unfortunate type signature. @JeffBezanson, are you concerned about this?

@JeffBezanson
Copy link
Member

I think [] is a really different case because that will be written by hand to initialize an array for pushing things into.

I'm concerned about it, but I also don't see what else can be done. Vector{Union()} is much better than Vector{Any} or Vector{String} in the empty map case.

@nalimilan
Copy link
Member

But won't the same problem reappear if you try to call push! on the Vector{Union()} resulting from calling map on an empty array? I imagine this could create run-time exceptions, but only under very specific conditions -- i.e. they would often not be detected by the developer until a user encounters the bug. So in a sense it might be worse than [], which at least gives this behavior more reliably.

@simonster
Copy link
Member

The counterpoint to @nalimilan's example is g(map(f, x)) where g takes a Vector{T<:Number}. This will work if map returns a Vector{Union()}, but won't work if it returns a Vector{Any}.

@nalimilan
Copy link
Member

Yeah, the eternal covariance vs. contravariance dilemma...

@JeffBezanson
Copy link
Member

When there is no "one right answer", you have to make these decisions based on expected use in each case. You hardly ever see code that even mutates, let alone grows, the result of a map. Unlike map, [] shows the specific intention to start with an empty array, while getting an empty array from map is just one case.

If arbitrarily mutating the result of map is the requirement, then you can only really give an Array{Any}.

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

No branches or pull requests

6 participants