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

make [a, b] not concatenate #3737

Closed
SamChill opened this issue Jul 16, 2013 · 34 comments
Closed

make [a, b] not concatenate #3737

SamChill opened this issue Jul 16, 2013 · 34 comments
Labels
kind:breaking This change will break code needs decision A decision on this change is needed
Milestone

Comments

@SamChill
Copy link
Contributor

This works:

julia> [ 1:3, 4 ]
4-element Int64 Array:
 1
 2
 3
 4

and this works:

julia> Uint8[ 1:3 ]
3-element Uint8 Array:
 0x01
 0x02
 0x03

but this doesn't:

julia> Uint8[ 1:3, 4]
ERROR: no method convert(Type{Uint8},Range1{Int64})
 in setindex! at array.jl:392
 in getindex at array.jl:161

Is this expected behavior? I would expect to be able to construct Uint8 arrays this way.

@simonster
Copy link
Member

If you specify the type of an array when constructing it, Julia doesn't attempt to vcat its elements. Your code won't work for Int64 either:

julia> Int64[1:3, 4]
ERROR: no method convert(Type{Int64},Range1{Int64})
 in setindex! at array.jl:403
 in getindex at array.jl:163

I think you want

julia> [uint8(1):uint8(3), uint8(4)]
4-element Uint8 Array:
 0x01
 0x02
 0x03
 0x04

@JeffBezanson
Copy link
Sponsor Member

It was supposed to be the case that T[x, y, z] made an array of element type T, so x, y, and z would each be converted to type T, and indeed you can't convert an integer range to type Uint8. Note that convert is very specific in this sense; converting to Uint8 means getting a Uint8 back, not an array of Uint8 or a range of Uint8 etc.

Then the waters were muddied a bit by the definition (array.jl:173) that allows Int[1:3]. I probably allowed that change at the time, and I deeply regret it. You would think constructing an array with a certain element type would be a simple enough matter.

We could go farther down this road and allow T[...] for all concatenations, but actually I'd prefer to remove all concatenating behavior from [] except for [a b] and [a; b] etc.

@StefanKarpinski
Copy link
Sponsor Member

We could go farther down this road and allow T[...] for all concatenations, but actually I'd prefer to remove all concatenating behavior from [] except for [a b] and [a; b] etc.

I still think this is probably a good idea. It would expand and clarify the distinction between [x,y,z] and [x y z].

@toivoh
Copy link
Contributor

toivoh commented Jul 18, 2013

+1

@ViralBShah
Copy link
Member

I would actually love to remove all concatenating behaviour apart for [a b] and [a; b] too. I think this is one syntax choice in our design that is inelegant.

@JeffBezanson
Copy link
Sponsor Member

It appears to be unanimous!
Unfortunately it will be really hard to track down all uses of [a,b] that rely on concatenation.

@toivoh
Copy link
Contributor

toivoh commented Jul 18, 2013

Deprecation warnings for concatenation could be a good start at least.

@StefanKarpinski
Copy link
Sponsor Member

We can turn them into a call to vcat_deprecated and print a warning. At some point we'll have eliminated all such calls and we can change the behavior to straight up fail and/or just not concatenate.

@mlubin
Copy link
Member

mlubin commented Jul 18, 2013

What's the replacement for concatenating vcat?

@mlubin
Copy link
Member

mlubin commented Jul 18, 2013

And how does this fix the issue:

julia> Uint8[1:3;4]
ERROR: no method convert(Type{Uint8},Range1{Int64})
 in setindex! at array.jl:415
 in getindex at array.jl:163

@StefanKarpinski
Copy link
Sponsor Member

I would allow Uint8[1:3;4] to do concatenation while Uint[1:3,4] would fail because 1:3 can't be converted to a Uint8. So it doesn't fix this issue without further changes, but makes it sane to do those changes.

@JeffBezanson
Copy link
Sponsor Member

And the question is whether Uint8[1:3] has zero commas or zero semicolons :)

@StefanKarpinski
Copy link
Sponsor Member

Ah yes, that is an issue too. Drat.

@JeffBezanson
Copy link
Sponsor Member

Color me amazed that the idea of making Uint8[x] always return a 1-element Uint8 array is so contentious.

@JeffBezanson JeffBezanson reopened this Jul 18, 2013
@JeffBezanson
Copy link
Sponsor Member

Oops closed wrong issue.

@StefanKarpinski
Copy link
Sponsor Member

Honestly, I'm fine with it.

@toivoh
Copy link
Contributor

toivoh commented Jul 19, 2013

How about letting Uint8[x] have zero commas, and requiring Uint8[x;] if you want zero semicolons (e.g. Uint8[1:3;])? Besides preferring not concatenate in general usage, it would let the A[x] case be handled by the same getindex code as A[x,y].

@JeffBezanson
Copy link
Sponsor Member

Probably the biggest issue is that [1:n] is really convenient for expanding ranges.

@StefanKarpinski
Copy link
Sponsor Member

Even though [a,b,c] and T[a,b,c] feel like the same syntax, they're really quite different. In particular there is no T you can pick to express the former.

nolta added a commit that referenced this issue Jul 20, 2013
@nolta
Copy link
Member

nolta commented Jul 22, 2013

Ok, i've implemented both options:

  1. [x] is equivalent to [x,] on mn/sanecat
  2. [x] is equivalent to [x;] on mn/sanecat3

Personally, i prefer (1), and would rather do nothing than (2).

@vtjnash
Copy link
Sponsor Member

vtjnash commented Aug 8, 2013

The style-guide section of the manual http://docs.julialang.org/en/latest/manual/style-guide/#don-t-overuse will need to be updated when this behavior is changed.

@JeffBezanson
Copy link
Sponsor Member

I have a growing urge to merge the mn/sanecat branch.

@StefanKarpinski
Copy link
Sponsor Member

What about the mn/sanecat3 branch?

@JeffBezanson
Copy link
Sponsor Member

The feeling is that concatenation is the "weird" behavior, so we shouldn't suddenly switch to it when n=1: [x,y,z], [x,y], and [x] should all do the same kind of thing.

@JeffBezanson
Copy link
Sponsor Member

Although I admit that n=1 also has a "feel" of its own, which makes [1:3] = [1,2,3] make sense, but [1:3, 1:4] giving a 2-element array of ranges also seem to make sense.

@JeffBezanson
Copy link
Sponsor Member

Also, while the [1:n] syntax is lovely, the operation it describes is not something that's highly encouraged. Ranges should be expanded to dense vectors as rarely as possible.

@milktrader
Copy link
Contributor

I found myself needing an array of ranges and blithely began constructing it

julia> [1:2, 4:8]
7-element Array{Int64,1}:
 1
 2
 4
 5
 6
 7
 8

Only to discover that I was getting concatenation for free, but it's not what I was expecting. typeof(1:2) is in fact Range1{Int} so the extra step needed to get the expected result is to do this:

julia> Range1[1:2, 4:8]
2-element Array{Range1{T<:Real},1}:
 1:2
 4:8

@jiahao
Copy link
Member

jiahao commented Jun 28, 2014

Just encountered this with generating array indices. The silent expansion of ranges is really quite confusing and a potential performance gotcha.

A=randn(5,5)
A[[1:2, 4:5], :] #4x5 Array{Float64,2}
A[[], :] #0x5 Array{Float64,2}
A[Range[1:2,4:5], :] #no method error
A[Range[], :] #no method error

@xianrenb
Copy link

xianrenb commented Nov 1, 2014

I believe we need to be able to create array of ranges in a concise way. For example, "[1:5, 1:3]" should be an array of two ranges: "1 to 5" and "1 to 3".
Range(s) between "[" and "]" should still be range(s).
Currently, we could convert range to array: "convert(Array, 1:5)" would give "[1, 2, 3, 4, 5]".
I would suggest new syntax like "1..5" to represent "[1, 2, 3, 4, 5]".
The following would become true:
"reverse(1..5)" represents "[5, 4, 3, 2, 1]".
"1..99[1:2:end]" represents "[1, 3, 5, 7, ..., 99]".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:breaking This change will break code needs decision A decision on this change is needed
Projects
None yet
Development

No branches or pull requests