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

Fix float range behavior #2959

Merged
merged 1 commit into from
Apr 28, 2013
Merged

Conversation

simonster
Copy link
Member

Previously:

1.0:1.5      # == 1.0:2.0
1.0:1.0:1.5  # == 1.0:1.0

With this patch:

1.0:1.5      # == 1.0:1.0
1.0:1.0:1.5  # == 1.0:1.0

I don't think the previous behavior was intentional, since it seems to violate the principle of least surprise, but I could be wrong.

@JeffBezanson
Copy link
Member

If we want x:y and x:1.0:y to always be equivalent, I believe the only option is to define

colon{T<:FloatingPoint}(start::T, stop::T) = colon(start, 1.0, stop)

But it also makes sense to insist on a Range1 from 2-argument colon, in which case your change is fine.

@simonster
Copy link
Member Author

I'd be fine with returning a Range, but if we want to return a Range1, maybe we could do something like:

function colon{T<:Real}(start::T, stop::T)
    if stop < start
        len = 0
    else
        nf = stop - start + 1
        if T <: FloatingPoint
            n = round(nf)
            len = abs(n-nf) < eps(n)*3 ? itrunc(n) : itrunc(nf)
        else
            n = nf
            len = iround(n)
        end
        if n >= typemax(Int)
            error("Range: length ",n," is too large")
        end
    end
    Range1(start, len)
end

Not exactly the same as x:1.0:y, but more intuitive than what's currently in this PR for cases like 1.0:(.3-.1)/.1

@JeffBezanson
Copy link
Member

Let's go with that, except the last len = iround(n) should be len = itrunc(n). That change is also needed in the existing 3-argument colon.

@simonster
Copy link
Member Author

Done.

@JeffBezanson
Copy link
Member

Great, thanks!

JeffBezanson added a commit that referenced this pull request Apr 28, 2013
@JeffBezanson JeffBezanson merged commit d121c4d into JuliaLang:master Apr 28, 2013
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

Successfully merging this pull request may close these issues.

2 participants