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

change int(::Float64) to error on non-integral values #6211

Closed
StefanKarpinski opened this issue Mar 19, 2014 · 15 comments
Closed

change int(::Float64) to error on non-integral values #6211

StefanKarpinski opened this issue Mar 19, 2014 · 15 comments
Labels
breaking This change will break code docs This change adds or pertains to documentation needs decision A decision on this change is needed
Milestone

Comments

@StefanKarpinski
Copy link
Member

We've discussed this before and I think it's a good change. This forces people to be explicit about whether they want rounding, truncation, or whatever. The current behavior is a gotcha for people coming from C. Related to which, do we have a document somewhere for people coming from C where we can mention that int/int = float, since that is a surprise when coming from a static language?

@StefanKarpinski StefanKarpinski changed the title change int(::Float64) to raise an error for non-integer values change int(::Float64) to error on non-integral values Mar 19, 2014
@jiahao
Copy link
Member

jiahao commented Mar 19, 2014

Perhaps the Noteworthy Differences from Other Languages section of the manual?

@StefanKarpinski
Copy link
Member Author

Ah, yes. I was wondering where that section had gone. We should probably have a C section in there.

@JeffBezanson
Copy link
Member

See also #5413.
To me the value of int(x) is that it's short and convenient. It's not so bad just to say that it does rounding.

@ivarne
Copy link
Member

ivarne commented Mar 19, 2014

I also think int(x) is an obvious sign that I really want to get a integer back. We have isinteger(x) if you think a non exact conversion deserves an error.

Wouldn't throwing an exception be a major speed bump for such a simple function?

@StefanKarpinski
Copy link
Member Author

I've encountered numerous people coming from static languages who were very surprised that we (a) don't do integer division and (b) do rounding instead of truncation. My justifications are weak. People do, however, seem to agree that requiring the programmer to be explicit about how they want to convert to integers is ok.

@JeffBezanson
Copy link
Member

Making things unsurprising to people used to static languages doesn't seem like the right standard at this point.
Rounding is a better default choice. It hasn't been the default in the past because it's slightly more expensive.

@ivarne
Copy link
Member

ivarne commented Mar 19, 2014

Integer division is just so annoying in python. I regularly waste 30 minutes before realising that both my variables happened to be integers. I don't have a strong opinion on the rounding vs truncation, but rounding is definitely less surprising.

@lindahua
Copy link
Contributor

We have ifloor, iceil, iround, and itrunc. All of these produce an integer given a floating-point value. I don't think they are more difficult to use than int. We should also encourage people to use these functions instead of int to explicitly express their intentions.

@lindahua
Copy link
Contributor

My feeling is that whenever you have more than one ways to define something and none is "obviously more correct" than others, then we should just don't allow that and tell people to articulate what they want explicitly.

@JeffBezanson
Copy link
Member

I think the real goal is to get rid of the int function (and its cohorts). That's something I can support. Then we'd just have Int(x) (which is convert), and iround etc.

@lindahua
Copy link
Contributor

@JeffBezanson I agree with this plan.

@StefanKarpinski
Copy link
Member Author

Excellent, we all agree.

@JeffBezanson
Copy link
Member

I realized there is an important connection between convert and how integer arithmetic works. We have Int32+Int32=Int64, but if you want to work entirely within 32-bit ints and do everything mod 2^32 you can get that by declaring variables:

local a::Int32, b::Int32
...
a = b+1

This works because convert silently truncates. If convert errored on overflow, then instead of 32-bit arithmetic you would have 32-bit-or-error arithmetic. It would become really hard to get standard 32-bit overflow arithmetic. You'd have to put itrunc everywhere, which is not workable.

So I would say if convert is changed to error on integer overflow (which would be reasonable), then arithmetic should be changed so Int32+Int32=Int32. (Of course this also has benefits for reductions, and consistency with code that writes to arrays.)

@StefanKarpinski
Copy link
Member Author

I've come to be in favor of both changes. The connection is interesting. It means that we can't make one change without the other.

@JeffBezanson
Copy link
Member

@jakebolewski points out that now we can do Array{Int}(x) where x is some other type of array. Since it uses convert this actually works already.

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 docs This change adds or pertains to documentation needs decision A decision on this change is needed
Projects
None yet
Development

No branches or pull requests

5 participants