-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Fold linspace into range, add keyword arguments, deprecate logspace #25896
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
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.0Any 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. |
mbauman
left a 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.
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
linspaceand its constructed typeLinSpacetolinrangeandLinRange, respectively. These names more accurately reflect what's being constructed.It's likely that the name
linspacewas 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 namelinspaceis 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 itlinrangeis more indicative of the functionality.Deprecate
logspacetologrange. 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.