Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "DataAPI"
uuid = "9a962f9c-6df0-11e9-0e5d-c546b8b5ee8a"
authors = ["quinnj <[email protected]>"]
version = "1.15.0"
version = "1.16.0"

[compat]
julia = "1"
Expand Down
109 changes: 107 additions & 2 deletions src/DataAPI.jl
Original file line number Diff line number Diff line change
Expand Up @@ -472,15 +472,17 @@ end
colmetadatakeys(x, [col])

If `col` is passed return an iterator of metadata keys for which
`metadata(x, col, key)` returns a metadata value. Throw an error if `x` does not
`colmetadata(x, col, key)` returns a metadata value. Throw an error if `x` does not
support reading column metadata or if `col` is not a column of `x`.

`col` must have a type that is supported by table `x` for column indexing.
Following the Tables.jl contract `Symbol` and `Int` are always allowed.

If `col` is not passed return an iterator of `col => colmetadatakeys(x, col)`
pairs for all columns that have metadata, where `col` are `Symbol`.
If `x` does not support column metadata return `()`.

If `colmetadatasupport(typeof(x)).read` or `colmetadatasupport(typeof(x)).write` return `true`
this should also be defined.
"""
function colmetadatakeys end

Expand Down Expand Up @@ -515,6 +517,109 @@ Throw an error if `x` does not support metadata deletion for column `col`.
"""
function emptycolmetadata! end

"""
dimmetadatasupport(T::Type, dim::Int)

Return a `NamedTuple{(:read, :write), Tuple{Bool, Bool}}` indicating whether
values of type `T` support metadata correspond to dimension `dim`.
Comment on lines +525 to +528
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I missed this substantial point: colmetadatasupport takes a single argument, i.e. it asks whether column metadata is supported at all, without considering a particular column. Shouldn't we do the same here? Once a type supports dimension metadata, it can return nothing for dimensions that do not have any metadata.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is could be OK as is proposed here. colmetadatasupport takes one argument, because there is exactly one dimension always.
But what @nalimilan is also OK - it depends on what @Tokazama thinks is better (i.e. do we imagine that only selected dimensions would have metadata?).

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by "there is exactly one dimension always"? :-)

Copy link
Member

Choose a reason for hiding this comment

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

OK I get it now, you mean that columns are the second dimension so colmetadatasupport(T) is similar to dimmetadatasupport(T, 2).

I still think there's a problem though: dimmetadata(x) returns a tuple of metadata for each dimension. But how can you check whether you can call dimmetadata(x) without getting an error? For that you would need to be able to call dimmetadatasupport(T).


The `read` field indicates whether reading metadata with the [`dimmetadata`](@ref)
and [`dimmetadatakeys`]](@ref) functions is supported.

The `write` field indicates whether modifying metadata with the [`dimmetadata!`](@ref),
[`deletemetadata!`](@ref), and [`emptymetadata!`](@ref) functions is supported.
"""
dimmetadatasupport(::Type, i::Int) = (read=false, write=false)

"""
dimmetadata(x, dim::Int, key::AbstractString, [default]; style::Bool=false)

Return metadata value associated with `x` for dimension `dim` and key `key`.
Throw an error if `x` does not support reading metadata for dimension `dim` or `x`
supports reading metadata, but does not have a mapping for dimension `dim` for `key`.

If `style=true` return a tuple of metadata value and metadata style. Metadata
style is an additional information about the kind of metadata that is stored for
the `key`.

$STYLE_INFO

If `default` is passed then return it if `x` supports reading metadata and has
dimension `dim` but mapping for `key` is missing.
If `style=true` return `(default, :default)`.
"""
function dimmetadata end

"""
dimmetadata(x, dim::Int; style::Bool=false)

Return a dictionary of metadata corresponding to dimension `dim` of object `x`. If `dim`
is not passed return a collection mapping each dimension to its metadata so that
`collection[dim][key] == dimmetadata(x, dim, key)`.
Throw an error if `x` does not support reading metadata corresponding to dimension `dim`.

If `style=true` values are tuples of metadata value and metadata style. Metadata
style is an additional information about the kind of metadata that is stored for
the `key`.

$STYLE_INFO

The returned object may be freshly allocated on each call to `dimmetadata` and
is considered to be owned by `x` so it must not be modified.
"""
function dimmetadata(x::T, dim::Int; style::Bool=false) where {T}
if !dimmetadatasupport(T, dim).read
throw(ArgumentError("Objects of type $T do not support reading dimension metadata"))
end
return Dict(key => dimmetadata(x, dim, key, style=style) for key in dimmetadatakeys(x, dim))
end
function dimmetadata(x::T; style::Bool=false) where {T}
Tuple(dimmetadata(x, dim; style=style) for dim in 1:ndims(x))
end

"""
dimmetadatakeys(x, [dim::Int])

If `dim` is passed return an iterator of metadata keys for which
`dimmetadata(x, dim, key)` returns a metadata value. Throw an error if `x` does not
support reading dimension metadata or if `dim` is not a dimension of `x`.

If `dim` is not passed return an iterator of `dim => dimmetadatakeys(x, dim)`
pairs for all dimensions that have metadata.

Copy link
Member

Choose a reason for hiding this comment

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

Let's do the same as for colmetadatakeys. Can you update the implementation accordingly?

Suggested change
If `x` does not support dimension metadata return `()`.

If `dimmetadatasupport(typeof(x)).read` or `dimmetadatasupport(typeof(x)).write` return `true`
this should also be defined.
"""
function dimmetadatakeys end

"""
dimmetadata!(x, dim::Int, key::AbstractString, value; style::Symbol=:default)

Set metadata for `x` for dimension `dim` for key `key` to have value `value`
and style `style` (`:default` by default) and return `x`.
Throw an error if `x` does not support setting metadata for dimension `dim`.

$STYLE_INFO
"""
function dimmetadata! end

"""
deletedimmetadata!(x, dim::Int, key::AbstractString)

Delete metadata corresponding to dimension `dim` of `x` for key `key` and return `x`
(if metadata for `key` is not present do not perform any action).
Throw an error if `x` does not support metadata deletion for dimension `dim`.
"""
function deletedimmetadata! end

"""
emptydimmetadata!(x, dim::Int)

Delete all metadata for `x` corresponding to dimension `dim` and return `x`.
Throw an error if `x` does not support metadata deletion for dimension `dim`.
"""
function emptydimmetadata! end

"""
rownumber(row)

Expand Down
78 changes: 77 additions & 1 deletion test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,23 @@ DataAPI.levels(x::TestArray) = reverse(DataAPI.levels(x.x))
struct TestMeta
table::Dict{String, Any}
col::Dict{Symbol, Dict{String, Any}}
dims::Tuple{Dict{String, Any}, Dict{String, Any}}

TestMeta() = new(Dict{String, Any}(), Dict{Symbol, Dict{String, Any}}())
function TestMeta()
new(Dict{String, Any}(), Dict{Symbol, Dict{String, Any}}(), (Dict{String, Any}(), Dict{String, Any}()))
end
end
Base.ndims(::TestMeta) = 2

DataAPI.metadatasupport(::Type{TestMeta}) = (read=true, write=true)
DataAPI.colmetadatasupport(::Type{TestMeta}) = (read=true, write=true)
function DataAPI.dimmetadatasupport(::Type{TestMeta}, dim::Int)
if dim == 1 || dim == 2
(read=true, write=true)
else
(read=false, write=false)
end
end

function DataAPI.metadata(x::TestMeta, key::AbstractString; style::Bool=false)
return style ? x.table[key] : x.table[key][1]
Expand Down Expand Up @@ -93,6 +104,38 @@ end

DataAPI.emptycolmetadata!(x::TestMeta) = empty!(x.col)

function DataAPI.dimmetadata(x::TestMeta, dim::Int, key::AbstractString; style::Bool=false)
return style ? x.dims[dim][key] : x.dims[dim][key][1]
end

function DataAPI.dimmetadata(x::TestMeta, dim::Int, key::AbstractString, default; style::Bool=false)
haskey(x.dims[dim], key) && return DataAPI.metadata(x, dim, key, style=style)
return style ? (default, :default) : default
end

function DataAPI.dimmetadatakeys(x::TestMeta, dim::Int)
if DataAPI.dimmetadatasupport(TestMeta, dim).read
return keys(x.dims[dim])
else
return ()
end
Comment on lines +117 to +121
Copy link
Member

Choose a reason for hiding this comment

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

The docstring says we should throw an error when dim doesn't exist. Can you test this behavior?
@bkamins We should probably also change colmetadatakeys?

Suggested change
if DataAPI.dimmetadatasupport(TestMeta, dim).read
return keys(x.dims[dim])
else
return ()
end
return keys(x.dims[dim])

end

function DataAPI.dimmetadata!(x::TestMeta, dim::Int, key::AbstractString, value; style::Symbol=:default)
x.dims[dim][key] = (value, style)
return x
end

function DataAPI.deletedimmetadata!(x::TestMeta, dim::Int, key::AbstractString)
delete!(x.dims[dim], key)
return x
end
function DataAPI.emptydimmetadata!(x::TestMeta, dim::Int)
empty!(x.dims[dim])
return x
end


# An example implementation of a table (without the Tables.jl interface)
# for testing DataAPI.rownumber
struct TestTable{T}
Expand Down Expand Up @@ -301,12 +344,23 @@ end
@test_throws MethodError DataAPI.colmetadatakeys(1, 1)
@test_throws MethodError DataAPI.colmetadatakeys(1)

@test_throws MethodError DataAPI.dimmetadata!(1, 1, "a", 10, style=:default)
@test_throws MethodError DataAPI.deletedimmetadata!(1, 1, "a")
@test_throws MethodError DataAPI.emptydimmetadata!(1, 1)
@test_throws MethodError DataAPI.dimmetadata(1, 1, "a")
@test_throws ArgumentError DataAPI.dimmetadata(1, 1)
@test_throws MethodError DataAPI.dimmetadata(1, 1, "a", style=true)
@test_throws ArgumentError DataAPI.dimmetadata(1, 1, style=true)

@test DataAPI.metadatasupport(Int) == (read=false, write=false)
@test DataAPI.colmetadatasupport(Int) == (read=false, write=false)
@test DataAPI.dimmetadatasupport(Int, 1) == (read=false, write=false)

tm = TestMeta()
@test DataAPI.metadatasupport(TestMeta) == (read=true, write=true)
@test DataAPI.colmetadatasupport(TestMeta) == (read=true, write=true)
@test DataAPI.dimmetadatasupport(TestMeta, 1) == (read=true, write=true)
@test DataAPI.dimmetadatasupport(TestMeta, 0) == (read=false, write=false)

@test isempty(DataAPI.metadatakeys(tm))
@test DataAPI.metadata(tm) == Dict()
Expand Down Expand Up @@ -353,6 +407,28 @@ end
@test DataAPI.colmetadata!(tm, :col, "a", "100", style=:note) == tm
DataAPI.emptycolmetadata!(tm)
@test isempty(DataAPI.colmetadatakeys(tm))

@test isempty(DataAPI.dimmetadatakeys(tm, 1))
@test isempty(DataAPI.dimmetadatakeys(tm)[1])
@test DataAPI.dimmetadata(tm, 1) == Dict()
@test DataAPI.dimmetadata(tm) == (Dict(), Dict())
@test DataAPI.dimmetadata(tm, 1, style=true) == Dict()
@test DataAPI.dimmetadata!(tm, 1, "a", "100", style=:note) == tm
@test collect(DataAPI.dimmetadatakeys(tm, 1)) == ["a"]
@test_throws KeyError DataAPI.dimmetadata(tm, 1, "b")
@test DataAPI.dimmetadata(tm, 1, "b", 123) == 123
@test_throws KeyError DataAPI.dimmetadata(tm, 1, "b", style=true)
@test DataAPI.dimmetadata(tm, 1, "b", 123, style=true) == (123, :default)
@test DataAPI.dimmetadata(tm, 1, "a") == "100"
@test DataAPI.dimmetadata(tm, 1) == Dict("a" => "100")
@test DataAPI.dimmetadata(tm)[1] == Dict("a" => "100")
@test DataAPI.dimmetadata(tm, 1, "a", style=true) == ("100", :note)
@test DataAPI.dimmetadata(tm, 1, style=true) == Dict("a" => ("100", :note))
DataAPI.deletedimmetadata!(tm, 1, "a")
@test isempty(DataAPI.dimmetadatakeys(tm, 1))
@test DataAPI.dimmetadata!(tm, 1, "a", "100", style=:note) == tm
DataAPI.emptydimmetadata!(tm, 1)
@test isempty(DataAPI.dimmetadatakeys(tm, 1))
end

@testset "rownumber" begin
Expand Down