Skip to content

Conversation

@kshyatt
Copy link
Member

@kshyatt kshyatt commented Jul 13, 2018

refs too

@kshyatt kshyatt added docs This change adds or pertains to documentation collections Data structures holding multiple items, e.g. sets labels Jul 13, 2018
@kshyatt kshyatt requested a review from StefanKarpinski July 13, 2018 19:54
@kshyatt
Copy link
Member Author

kshyatt commented Jul 13, 2018

oh yeah the doc tests passed locally for me

@kshyatt
Copy link
Member Author

kshyatt commented Jul 14, 2018

Anyone feel like reviewing this?

base/range.jl Outdated
Supertype for ordinal ranges with elements of type `T` with
spacing(s) of type `S`.
An ordinal range is one whose values have a well-defined ordering.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be true, but I think it should be true of all ranges. I think the key property that we're going after here — and the reasoning behind the word Ordinal in this name — is that its subtypes have always-exact steps that are multiples of oneunit, and T should be a "discrete" datatype where it's not possible to represent values smaller than oneunit.

I think. Can anyone back me up on this?

base/range.jl Outdated
"""
AbstractUnitRange{T} <: OrdinalRange{T, T}
Supertype for unit ranges with elements of type `T`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer ranges with a step size of [`oneunit`](@ref)` over "unit ranges".

base/range.jl Outdated
Ranges with elements of type `T` with spacing of type `S`. The step
between each element is constant, and the range is defined in terms
of a `start` and `stop` of type `T` and a `step` of type `S`. Neither
`T` nor `S` should be floating point types.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a note that the syntax 1:2:10 with integer values creates StepRanges?

UnitRange{T<:Real}
A range parameterized by a `start` and `stop` of type `T`, filled
with elements spaced by `1` from `start` until `stop` is exceeded.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The syntax x:y with integer values x and y creates a UnitRange.

Copy link
Member

@mbauman mbauman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last comment and then this is good to go. Thanks!

base/range.jl Outdated
Supertype for ordinal ranges with elements of type `T` with
spacing(s) of type `S`.
The spacing of the elements of the range does not need to be even. The steps
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what "The spacing of the elements of the range does not need to be even" means. Maybe just remove this sentence?

@kshyatt
Copy link
Member Author

kshyatt commented Jul 17, 2018

OK I deleted the comment! And CI passed! Gonna merge :)

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

Labels

collections Data structures holding multiple items, e.g. sets docs This change adds or pertains to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants