Skip to content

Conversation

@stevengj
Copy link
Member

@stevengj stevengj commented Dec 23, 2016

This PR partially addresses #7903: it makes numbers non-indexable, so that they no longer act like 0-dimensional arrays (no size, getindex, ndims, etc). (broadcast is unaffected.)

  • Pro: being able to do 1[1] was always a bit odd, and it doesn't seem to gain us much. Eliminating this functionality seems to require minimal changes to Base. Also, the f.(x) syntax removes some of the impetus to write functions where scalars can be treated like arrays.

  • Con: eliminating this functionality doesn't actually save us much code in Base, and it might make some kinds of generic functions more annoying to write, especially since numbers are still iterable. Is it worth it?

I initially also tried making numbers non-iterable (removing start etc.). However, I quickly found that there is a lot of code in Base that is affected, and would seem to become much uglier without this capability (see #7903 (comment)). My conclusion is that making numbers iterable is more likely something we want to retain.

@stevengj stevengj added the breaking This change will break code label Dec 23, 2016
@yuyichao
Copy link
Contributor

OTOH all of the related hard to debug issues I've had are caused by numbers being iterable though..... (they are basically all for i in n instead of for i in 1:n....)

@stevengj
Copy link
Member Author

Even if we do eventually want to make numbers non-iterable (which I'm more skeptical of now that I see how much inconvenience it will cause in Base), removing getindex would be a good incremental first step.

@tkelman
Copy link
Contributor

tkelman commented Dec 23, 2016

shouldn't it be deprecated first, rather than immediately removed?

@tkelman tkelman added the needs news A NEWS entry is required for this change label Dec 23, 2016
@stevengj
Copy link
Member Author

stevengj commented Dec 23, 2016

@tkelman, right, I need to add deprecations and NEWS, but I wanted to check first whether we even want to do this.

@stevengj
Copy link
Member Author

Added deprecations and NEWS.

@stevengj stevengj removed the needs news A NEWS entry is required for this change label Dec 27, 2016
@stevengj
Copy link
Member Author

@StefanKarpinski, do you have an opinion on this?

@tkelman tkelman added the needs decision A decision on this change is needed label Dec 29, 2016
@stevengj
Copy link
Member Author

Rebased; bump.

@stevengj stevengj removed the breaking This change will break code label Dec 29, 2016
@stevengj
Copy link
Member Author

(Removing the breaking label since there is deprecation code.)

@iamed2
Copy link
Contributor

iamed2 commented Dec 29, 2016

Why not leave getindex(x::Number) = x?

@stevengj
Copy link
Member Author

@iamed2, if we leave that we should leave the rest, and not merge this. Which is fine with me; I don't have strong feelings about whether numbers should act as 0-dimensional arrays.

@JeffBezanson
Copy link
Member

The consensus on the triage call was leaning against this. I feel it's better to support all or none of the 0-d array interface, and, as has been observed, iteration tends to cause more problems here than indexing.

@kshyatt kshyatt added the deprecation This change introduces or involves a deprecation label Jan 8, 2017
@tpapp
Copy link
Contributor

tpapp commented Jan 21, 2017

I discovered in(x,x)==true accidentally (it is not documented in the docstring of in), thought it was neat, but then I did not realize that it only holds for numbers, before tracking down a bug in my code and reading the source. Asking on Discourse directed me here. I now understand the rationale behind this approach, but I think that the only two consistent approaches are

  1. make everything, not just numbers, a collection of a single element, with methods overriden by for actual collections,
  2. remove all collection-like behavior for numbers.

I find special treatment for numbers only confusing. I think that (1) would have ramifications that are surprising.

@JeffBezanson
Copy link
Member

I think that argument is a bit too pedantic. "Always" and "never" aren't the only two reasonable options; in fact we tend to prefer "as needed".

There is precedent for considering scalars to be "0-dimensional", and in practice we've wanted that behavior in many cases. As far as I know there has not been much need to extend this to other kinds of objects.

Of course your option (2) is popular, but trying to implement it has proven frustrating so far. I think there is still a chance we will eventually make this change though. In any case this PR doesn't do that, so I'll close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deprecation This change introduces or involves a deprecation needs decision A decision on this change is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants