-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fold linspace into range, add keyword arguments, deprecate logspace #25896
Conversation
I don't deny that Unless you want to create an actual |
I can remove the |
julia> range(1,2,5)
1:2:9
julia> linrange(1,2,5)
1.0:0.25:2.0 Any tips for remembering the difference between |
Oh right, I had forgotten that we still have |
Maybe rename |
+1 for renaming Regarding a potential |
Good idea to rename |
|
or |
That doesn't mean we can't define |
I think having |
Tell you what, I'll remove the |
I'd say keep it as is – I don't see what the problem is. Even if it's not a subtype of |
Okay, I'm fine with that too. |
Can |
More concretely, we could deprecate |
|
Triage likes the following plan:
|
421dbdf
to
7a493c2
Compare
Okay folks. There are now four distinct commits:
The tests pass and building documentation works locally, so we'll see what CI has to say about it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those _range
methods are indeed a little messy, but they're really not terrible. It might help to have a comment that describes an overall strategy and organize the methods based upon what's missing, what you're filling in, and what you're converting.
base/range.jl
Outdated
"one or the other")) | ||
end | ||
# We know that the passed values are consistent, so `length` is redundant | ||
colon(start, step, stop) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be a little safer to discard the step
and defer to _range(start::T, ::Nothing, stop::T, len::Integer)
. Steps are sloppy, lengths and endpoints can and should be exact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stop
and length
with no step
goes through the linspace
machinery though. Is that what we want here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that'd be my preference. unique(diff(r))
likely isn't going to be [step]
no matter what we do, but we can make the other three match exactly with a LinRange
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this case be an error, for over-specifying?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is an error if !isapprox((stop - start) / (length - 1), step)
(five lines up, just outside of the context window GitHub gives you on my comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should always be an error to call it this way. The isapprox
check is going to be unpredictable, and if you get that error how do you fix your code? Probably by removing one of the arguments.
Does this final api mean I would convert: I like the range name (has basically the same api as R's |
For 0.7 deprecation purposes, yes that is the API. But in 1.0 we can allow |
7a493c2
to
5431809
Compare
This name more accurately reflects what's being constructed. It's likely that the name `LinSpace` was chosen for Julia since it matches what's used by Matlab. NumPy uses this name as well, but they likely also chose it for Matlab familiarity. In Matlab, the name `linspace` is short for "linearly spaced," and it returns a vector of linearly spaced elements. In Julia, we're often more careful with our terminology: in this case, the returned object is an `AbstractRange`, not some kind of vector space or any other kind of space. Thus calling it `LinRange` is more indicative of the functionality.
5431809
to
a2cddff
Compare
CI failures are unrelated. |
Important to have Compat support for this. |
The `linspace` to `range` change is a bit awkward at the moment. Instead of `linspace(start,stop,len)`, we have to use `range(start, stop = stop, length = len)`. The `stop` keyword is only necessary in v0.7, and will [probably be relaxed](JuliaLang/julia#25896 (comment)) in v1.0
The `linspace` to `range` change is a bit awkward at the moment. Instead of `linspace(start,stop,len)`, we have to use `range(start, stop = stop, length = len)`. The `stop` keyword is only necessary in v0.7, and will [probably be relaxed](JuliaLang/julia#25896 (comment)) in v1.0
- info replaced by @info JuliaLang/julia#24490 -linspace has been deprecated in favor of range with stop and length keyword arguments (#25896). JuliaLang/julia#25896
- info replaced by @info JuliaLang/julia#24490 -linspace has been deprecated in favor of range with stop and length keyword arguments (#25896). JuliaLang/julia#25896
EDIT: The description below is outdated and reflects the original intent of the PR. Scroll down for current state.
This PR consists of two commits:
Deprecate
linspace
and its constructed typeLinSpace
tolinrange
andLinRange
, respectively. These names more accurately reflect what's being constructed.It's likely that the name
linspace
was chosen for Julia since it matches what's used by Matlab. NumPy uses this name as well, but they likely also chose it for Matlab familiarity. In Matlab, the namelinspace
is short for "linearly spaced," and it returns a vector of linearly spaced elements.In Julia, we're often more careful with our terminology: in this case, the returned object is an
AbstractRange
, not some kind of vector space or any other kind of space. Thus calling itlinrange
is more indicative of the functionality.Deprecate
logspace
tologrange
. While the accuracy of terminology argument doesn't apply as strongly to this function as it does forlinspace
, it still makes sense (at least to me) to make this name match itslin*
counterpart.