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

add default constructors for AbstractRange #48894

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

putianyi889
Copy link
Contributor

@putianyi889 putianyi889 commented Mar 4, 2023

Copy link
Member

@inkydragon inkydragon left a comment

Choose a reason for hiding this comment

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

Is it necessary to add constructors for abstract types?

julia> methods(AbstractRange)
# 0 methods for type constructor

julia> supertype(AbstractRange)
AbstractVector (alias for AbstractArray{T, 1} where T)

julia>  methods(AbstractVector)
# 0 methods for type constructor

julia> supertype(AbstractVector)
Any

@inkydragon inkydragon added the ranges Everything AbstractRange label Mar 10, 2023
@putianyi889
Copy link
Contributor Author

As you can see from added tests, it does eltype conversion for general ranges.
AbstractArray has constructors.

@putianyi889 putianyi889 requested a review from inkydragon March 10, 2023 16:15
putianyi889 added a commit to putianyi889/PTYQoL.jl that referenced this pull request Nov 5, 2023
@vtjnash vtjnash added the forget me not PRs that one wants to make sure aren't forgotten label Mar 21, 2024
@jishnub
Copy link
Contributor

jishnub commented Mar 22, 2024

@putianyi889 This seems like a good idea. Is this ready from your side?
We would probably want more tests.

@putianyi889
Copy link
Contributor Author

Is this ready from your side?

confirm.

We would probably want more tests.

not sure how many. maybe add 2-3 segments to every conversion test?

base/range.jl Outdated Show resolved Hide resolved
test/ranges.jl Outdated Show resolved Hide resolved
base/range.jl Outdated Show resolved Hide resolved
@jishnub
Copy link
Contributor

jishnub commented Aug 17, 2024

Could you add tests for other range types, such as StepRangeLen and StepRange, including cases where the elements are floating-point types?

base/range.jl Outdated Show resolved Hide resolved
putianyi889 added a commit to putianyi889/PTYQoL.jl that referenced this pull request Oct 30, 2024
@putianyi889
Copy link
Contributor Author

There are some issues with the step type of OrdinalRange, but I think that should be addressed in another issue.

putianyi889 added a commit to putianyi889/PTYQoL.jl that referenced this pull request Nov 1, 2024
@putianyi889
Copy link
Contributor Author

Found this

julia/base/abstractarray.jl

Lines 1237 to 1244 in 7635190

## range conversions ##
map(::Type{T}, r::StepRange) where {T<:Real} = T(r.start):T(r.step):T(last(r))
map(::Type{T}, r::UnitRange) where {T<:Real} = T(r.start):T(last(r))
map(::Type{T}, r::StepRangeLen) where {T<:AbstractFloat} = convert(StepRangeLen{T}, r)
function map(::Type{T}, r::LinRange) where T<:AbstractFloat
LinRange(T(r.start), T(r.stop), length(r))
end

@putianyi889 putianyi889 requested a review from jishnub November 15, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
forget me not PRs that one wants to make sure aren't forgotten ranges Everything AbstractRange
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants