-
Notifications
You must be signed in to change notification settings - Fork 33
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 parse method #392
base: master
Are you sure you want to change the base?
Add parse method #392
Conversation
Motivation: ``` julia> using CategoricalArrays julia> x = categorical(["2", "1"], ordered=true) 2-element CategoricalArray{String,1,UInt32}: "2" "1" julia> parse(Int64, x[1]) ERROR: MethodError: no method matching parse(::Type{Int64}, ::CategoricalValue{String, UInt32}) Closest candidates are: parse(::Type{T}, ::AbstractChar; base) where T<:Integer at C:\Users\Dennis Bal\.julia\juliaup\julia-1.7.1+0~x64\share\julia\base\parse.jl:40 parse(::Type{T}, ::AbstractString; base) where T<:Integer at C:\Users\Dennis Bal\.julia\juliaup\julia-1.7.1+0~x64\share\julia\base\parse.jl:240 parse(::Type{T}, ::AbstractString; kwargs...) where T<:Real at C:\Users\Dennis Bal\.julia\juliaup\julia-1.7.1+0~x64\share\julia\base\parse.jl:379 Stacktrace: [1] top-level scope @ REPL[176]:1 julia> parse.(Int64, x) ERROR: MethodError: no method matching parse(::Type{Int64}, ::CategoricalValue{String, UInt32}) Closest candidates are: parse(::Type{T}, ::AbstractChar; base) where T<:Integer at C:\Users\Dennis Bal\.julia\juliaup\julia-1.7.1+0~x64\share\julia\base\parse.jl:40 parse(::Type{T}, ::AbstractString; base) where T<:Integer at C:\Users\Dennis Bal\.julia\juliaup\julia-1.7.1+0~x64\share\julia\base\parse.jl:240 parse(::Type{T}, ::AbstractString; kwargs...) where T<:Real at C:\Users\Dennis Bal\.julia\juliaup\julia-1.7.1+0~x64\share\julia\base\parse.jl:379 Stacktrace: [1] _broadcast_getindex_evalf @ .\broadcast.jl:670 [inlined] [2] _broadcast_getindex @ .\broadcast.jl:653 [inlined] [3] getindex @ .\broadcast.jl:597 [inlined] [4] copy @ .\broadcast.jl:899 [inlined] [5] materialize(bc::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(parse), Tuple{Base.RefValue{Type{Int64}}, CategoricalVector{String, UInt32, String, CategoricalValue{String, UInt32}, Union{}}}}) @ Base.Broadcast .\broadcast.jl:860 [6] top-level scope @ REPL[177]:1 julia> import Base.parse julia> parse(T::DataType, catval::CategoricalValue) = parse(T, unwrap(catval)) parse (generic function with 17 methods) julia> parse(Int64, x[1]) 2 julia> parse.(Int64, x) 2-element Vector{Int64}: 2 1 ``` I am not sure if I added the code in the correct place, or if it makes more sense somewhere else. I also don't know if `Base.parse` would need to be explicitly imported. I just tried to do what has been done is the surrounding lines.
The only problem is that
and However, maybe we can add this as a convenience function. Let us wait for @nalimilan to comment. (as you have noted in your implementation it is already quite easy to achieve what you want just by using |
At some point |
I mean, there is also the And because the functionality is still the same (namely to take a text representation of a number and parse it as a literal), I find it completely appropriate to extend So to me, it is not a problem that As for other types T in Adding a single sentence "Base functions that operate on strings are extended to categorical values of type string" somewhere in the docs would make the entire thing transparent to the user, and I can not think of any reason why anyone would have an issue with this. |
Motivation:
I am not sure if I added the code in the correct place, or if it makes more sense somewhere else. I also don't know if
Base.parse
would need to be explicitly imported. I just tried to do what has been done is the surrounding lines.