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

Widen RangeIndex from Int to BitInteger #43262

Merged
merged 3 commits into from
Oct 26, 2022

Conversation

mkitti
Copy link
Contributor

@mkitti mkitti commented Nov 30, 2021

Could we widen RangeIndex to include indices that are a BitInteger?

@oscardssmith
Copy link
Member

This PR should have a test.

@mkitti
Copy link
Contributor Author

mkitti commented Oct 4, 2022

This PR should have a test.

Yes, thanks for reminding me. As you can see, I also am still trying to get the current tests to pass.

@mkitti
Copy link
Contributor Author

mkitti commented Oct 10, 2022

Are any of the test failures related?

@mkitti
Copy link
Contributor Author

mkitti commented Oct 25, 2022

Thanks for the rebase. I'm embarrassed by the poor branch naming here.

@mkitti
Copy link
Contributor Author

mkitti commented Oct 26, 2022

@StefanKarpinski, is there anything here for me to fix?

@StefanKarpinski StefanKarpinski merged commit 7cbf55f into JuliaLang:master Oct 26, 2022
@StefanKarpinski
Copy link
Member

Nope, all good! Thanks for the PR.

@@ -112,6 +112,8 @@ struct RefArray{T,A<:AbstractArray{T},R} <: Ref{T}
end
RefArray(x::AbstractArray{T}, i::Int, roots::Any) where {T} = RefArray{T,typeof(x),Any}(x, i, roots)
RefArray(x::AbstractArray{T}, i::Int=1, roots::Nothing=nothing) where {T} = RefArray{T,typeof(x),Nothing}(x, i, nothing)
RefArray(x::AbstractArray{T}, i::Integer, roots::Any) where {T} = RefArray{T,typeof(x),Any}(x, Int(i), roots)
RefArray(x::AbstractArray{T}, i::Integer=1, roots::Nothing=nothing) where {T} = RefArray{T,typeof(x),Nothing}(x, Int(i), nothing)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
RefArray(x::AbstractArray{T}, i::Integer=1, roots::Nothing=nothing) where {T} = RefArray{T,typeof(x),Nothing}(x, Int(i), nothing)
RefArray(x::AbstractArray{T}, i::Integer, roots::Nothing=nothing) where {T} = RefArray{T,typeof(x),Nothing}(x, Int(i), nothing)

I don't think you meant to overwrite the previous RefArray(x::AbstractArray{T}) method here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a new pull request in #47337

mkitti added a commit to mkitti/julia that referenced this pull request Oct 26, 2022
This removes the default argument in `RefArray(x::AbstractArray{T}, i::Integer=1)` such that it does not override `RefArray(x::AbstractArray{T}, i::Int=1)`

xref: JuliaLang#43262 (comment)
cc: @vtjnash
vtjnash pushed a commit that referenced this pull request Oct 28, 2022
This removes the default argument in `RefArray(x::AbstractArray{T}, i::Integer=1)`
such that it does not override `RefArray(x::AbstractArray{T}, i::Int=1)`.

Refs: #43262 (comment)
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.

4 participants