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.

Sign up for GitHub

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

Throw error in getindex function when string and symbol are passed simultaneously #197

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Throw error in getindex function when string and symbol are passed simultaneously #197

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0bc8081
Updating the getindex function to throw argument error when string an…
siddjain444 Apr 25, 2024
56440c4
Merge branch 'xKDR:main' into t175_julia_version
siddjain444 Apr 25, 2024
449b42c
Updating the getindex.jl to address the issue #194 and #190
siddjain444 Apr 25, 2024
986af3a
Making changes to getindex.jl to resolve the issue #190, #194
siddjain444 Apr 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 64 additions & 20 deletions src/getindex.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,12 @@ julia> ts[1, "x1"]; # same as above
###

### Inputs: row scalar, column scalar; Output: scalar

Copy link
Member

Choose a reason for hiding this comment

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

Can you remove the extra line here?

function Base.getindex(ts::TSFrame, i::Int, j::Int)
return ts.coredata[i,j+1]
end

function Base.getindex(ts::TSFrame, i::Int, j::Union{Symbol, String})
function Base.getindex(ts::TSFrame, i::Int, j::Union{Symbol, AbstractString})
chiraganand marked this conversation as resolved.
Show resolved Hide resolved
return ts.coredata[i, j]
end

Expand All @@ -265,29 +266,40 @@ function Base.getindex(ts::TSFrame, dt::T, j::Int) where {T<:TimeType}
ts.coredata[idx, j+1]
end

function Base.getindex(ts::TSFrame, dt::T, j::Union{String, Symbol}) where {T<:TimeType}
function Base.getindex(ts::TSFrame, dt::T, j::Union{AbstractString, Symbol}) where {T<:TimeType}
chiraganand marked this conversation as resolved.
Show resolved Hide resolved
idx = findfirst(x -> x == dt, index(ts))
ts.coredata[idx, j]
end
###

### Inputs: row scalar, column vector; Output: TSFrame
function Base.getindex(ts::TSFrame, i::Int, j::AbstractVector{Int})
TSFrame(ts.coredata[[i], Cols(:Index, j.+1)]) # increment: account for Index
TSFrame(ts.coredata[[i], Cols(:Index, j.+1)]) # increment: account for Index
Copy link
Member

Choose a reason for hiding this comment

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

There is only whitespace difference in this line. Can you remove this change too?

end

function Base.getindex(ts::TSFrame, i::Int, j::AbstractVector{T}) where {T<:Union{String, Symbol}}
TSFrame(ts.coredata[[i], Cols(:Index, j)])

function Base.getindex(ts::TSFrame, i::Int, j::AbstractVector{T}) where {T<:Union{AbstractString, Symbol}}
if length(unique(map(x -> typeof(x), j))) == 1
TSFrame(ts.coredata[[i], Cols(:Index, j)])
else
throw(ArgumentError("The column vector cannot contain both String and Symbol types."))
end
end


Copy link
Member

Choose a reason for hiding this comment

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

Empty line.

function Base.getindex(ts::TSFrame, dt::T, j::AbstractVector{Int}) where {T<:TimeType}
idx = findfirst(x -> x == dt, index(ts))
ts[idx, j]
end

function Base.getindex(ts::TSFrame, dt::D, j::AbstractVector{T}) where {D<:TimeType, T<:Union{String, Symbol}}
function Base.getindex(ts::TSFrame, dt::D, j::AbstractVector{T}) where {D<:TimeType, T<:Union{AbstractString, Symbol}}
idx = findfirst(x -> x == dt, index(ts))
ts[idx, j]
if length(unique(map(x -> typeof(x), j))) == 1
ts[idx, j]
else
throw(ArgumentError("The column vector cannot contain both AbstractString and Symbol types."))
end

end
###

Expand All @@ -302,7 +314,8 @@ function Base.getindex(ts::TSFrame, i::AbstractVector{Int}, j::Int)
ts.coredata[i, j+1] # increment: account for Index
end

function Base.getindex(ts::TSFrame, i::AbstractVector{Int}, j::Union{String, Symbol})

Copy link
Member

Choose a reason for hiding this comment

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

Extra line.

function Base.getindex(ts::TSFrame, i::AbstractVector{Int}, j::Union{AbstractString, Symbol})
Copy link
Member

Choose a reason for hiding this comment

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

Check is needed here.

ts.coredata[i, j]
end

Expand All @@ -314,12 +327,18 @@ function Base.getindex(ts::TSFrame, dt::AbstractVector{T}, j::Int) where {T<:Tim
ts[idx, j]
end

function Base.getindex(ts::TSFrame, dt::AbstractVector{T}, j::Union{String, Symbol}) where {T<:TimeType}

function Base.getindex(ts::TSFrame, dt::AbstractVector{T}, j::Union{AbstractString, Symbol}) where {T<:TimeType}
idx = map(d -> findfirst(x -> x == d, index(ts)), dt)
if length(idx) == 0
return nothing
else
if length(unique(map(x -> typeof(x), j))) == 1
Copy link
Member

Choose a reason for hiding this comment

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

This check should happen before idx = map(d -> ...).

ts[idx, j]
else
throw(ArgumentError("The column vector cannot contain both AbstractString and Symbol types."))
end
end
ts[idx, j]
end
###

Expand All @@ -328,10 +347,16 @@ function Base.getindex(ts::TSFrame, i::AbstractVector{Int}, j::AbstractVector{In
TSFrame(ts.coredata[i, Cols(:Index, j.+1)]) # increment: account for Index
end

function Base.getindex(ts::TSFrame, i::AbstractVector{Int}, j::AbstractVector{T}) where {T<:Union{String, Symbol}}
TSFrame(ts.coredata[i, Cols(:Index, j)])

Copy link
Member

Choose a reason for hiding this comment

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

Extra line.

function Base.getindex(ts::TSFrame, i::AbstractVector{Int}, j::AbstractVector{T}) where {T<:Union{AbstractString, Symbol}}
if length(unique(map(x -> typeof(x), j))) == 1
TSFrame(ts.coredata[i, Cols(:Index, j)])
else
throw(ArgumentError("The column vector cannot contain both AbstractString and Symbol types."))
end
end


Copy link
Member

Choose a reason for hiding this comment

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

Extra line.

function Base.getindex(ts::TSFrame, dt::AbstractVector{T}, j::AbstractVector{Int}) where {T<:TimeType}
idx = map(d -> findfirst(x -> x == d, index(ts)), dt)
if length(idx) == 0
Expand All @@ -340,9 +365,15 @@ function Base.getindex(ts::TSFrame, dt::AbstractVector{T}, j::AbstractVector{Int
ts[idx, j]
end

function Base.getindex(ts::TSFrame, dt::AbstractVector{D}, j::AbstractVector{T}) where {D<:TimeType, T<:Union{String, Symbol}}

Copy link
Member

Choose a reason for hiding this comment

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

Extra line.

function Base.getindex(ts::TSFrame, dt::AbstractVector{D}, j::AbstractVector{T}) where {D<:TimeType, T<:Union{AbstractString, Symbol}}
idx = map(d -> findfirst(x -> x == d, index(ts)), dt)
ts[idx, j]
if length(unique(map(x -> typeof(x), j))) == 1
Copy link
Member

Choose a reason for hiding this comment

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

The check should happen before idx = map(d -> ....

ts[idx, j]
else
throw(ArgumentError("The column vector cannot contain both AbstractString and Symbol types."))
end

end
###

Expand All @@ -362,7 +393,8 @@ function Base.getindex(ts::TSFrame, i::UnitRange, j::Int)
ts[collect(i), j]
end

function Base.getindex(ts::TSFrame, i::UnitRange, j::Union{String, Symbol})

Copy link
Member

Choose a reason for hiding this comment

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

Extra line.

function Base.getindex(ts::TSFrame, i::UnitRange, j::Union{AbstractString, Symbol})
ts[collect(i), j]
end
###
Expand All @@ -372,8 +404,14 @@ function Base.getindex(ts::TSFrame, i::UnitRange, j::AbstractVector{Int})
ts[collect(i), j]
end

function Base.getindex(ts::TSFrame, i::UnitRange, j::AbstractVector{T}) where {T<:Union{String, Symbol}}
ts[collect(i), j]

function Base.getindex(ts::TSFrame, i::UnitRange, j::AbstractVector{T}) where {T<:Union{AbstractString, Symbol}}
if length(unique(map(x -> typeof(x), j))) == 1
ts[collect(i), j]
else
throw(ArgumentError("The column vector cannot contain both AbstractString and Symbol types."))
end

end
###

Expand Down Expand Up @@ -476,7 +514,8 @@ function Base.getindex(ts::TSFrame, ::Colon, j::Int)
ts[1:TSFrames.nrow(ts), j]
end

function Base.getindex(ts::TSFrame, ::Colon, j::Union{String, Symbol})

Copy link
Member

Choose a reason for hiding this comment

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

Extra line.

function Base.getindex(ts::TSFrame, ::Colon, j::Union{AbstractString, Symbol})
ts[1:TSFrames.nrow(ts), j]
end
###
Expand All @@ -486,8 +525,13 @@ function Base.getindex(ts::TSFrame, ::Colon, j::AbstractVector{Int})
ts[1:TSFrames.nrow(ts), j]
end

function Base.getindex(ts::TSFrame, ::Colon, j::AbstractVector{T}) where {T<:Union{String, Symbol}}
ts[1:TSFrames.nrow(ts), j]
function Base.getindex(ts::TSFrame, ::Colon, j::AbstractVector{T}) where {T<:Union{AbstractString, Symbol}}
if length(unique(map(x -> typeof(x), j))) == 1
ts[1:TSFrames.nrow(ts), j]
else
throw(ArgumentError("The column vector cannot contain both AbstractString and Symbol types."))
end

Copy link
Member

Choose a reason for hiding this comment

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

Extra line.

end
###

Expand Down
Loading