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

Rename the internal union OneBasedRanges to _OneBasedRanges #54079

Merged
merged 2 commits into from
May 25, 2024

Conversation

jishnub
Copy link
Contributor

@jishnub jishnub commented Apr 14, 2024

This makes auto-completing Base.OneTo easier, as there's no ambiguity with this internal union. The leading underscore may also signal that the name is internal to Base.

@IanButterworth
Copy link
Member

Or we make autocomplete hints for private names a bit slower and require tab + another key combination.

@jishnub
Copy link
Contributor Author

jishnub commented Apr 15, 2024

That's certainly a better longer-term solution

@LilithHafner
Copy link
Member

We could also replace this internal union with a trait over types

is_one_based(::Type{AbstractArray}) = false
is_one_based(::Type{Array}) = true
is_one_based(::Type{OneTo}) = true
is_one_based(::Type{IdentityUnitRange{T}}) where T = is_one_based(T)
is_one_based(::AbstractArray) = throw(helpful error message)

and then also use it here

julia/base/genericmemory.jl

Lines 301 to 307 in f30aae5

"""
view(m::GenericMemory{M, T}, inds::Union{UnitRange, OneTo})
Create a vector `v::Vector{T}` backed by the specified indices of `m`. It is only safe to
resize `v` if `m` is subseqently not used.
"""
function view(m::GenericMemory{M, T}, inds::Union{UnitRange, OneTo}) where {M, T}

@fingolfin
Copy link
Member

I like the trait idea by @LilithHafner but perhaps that's for a future PR?

@fingolfin fingolfin added the merge me PR is reviewed. Merge when all tests are passing label May 24, 2024
@jishnub jishnub merged commit 2a5d553 into master May 25, 2024
8 checks passed
@jishnub jishnub deleted the jishnub/onebasedranges branch May 25, 2024 06:04
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label May 25, 2024
DilumAluthge pushed a commit that referenced this pull request Jun 3, 2024
This makes auto-completing `Base.OneTo` easier, as there's no ambiguity
with this internal union. The leading underscore may also signal that
the name is internal to `Base`.

Co-authored-by: Max Horn <max@quendi.de>
lazarusA pushed a commit to lazarusA/julia that referenced this pull request Jul 12, 2024
…aLang#54079)

This makes auto-completing `Base.OneTo` easier, as there's no ambiguity
with this internal union. The leading underscore may also signal that
the name is internal to `Base`.

Co-authored-by: Max Horn <max@quendi.de>
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.

5 participants