-
Notifications
You must be signed in to change notification settings - Fork 13
Suppor dimensional metadata #62
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
Open
Tokazama
wants to merge
26
commits into
JuliaData:main
Choose a base branch
from
Tokazama:dimmetadata
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
5ae5ab0
dimension metadata support
Tokazama 47daf4c
Update test/runtests.jl
Tokazama e1b08c5
Version bump and delete unnecessary line
Tokazama 19287e0
Make docs more consistent with colmetadata
Tokazama 10ca2f0
remove `dim`
Tokazama bbe4fc7
Update src/DataAPI.jl
Tokazama ffbb582
fix typos
Tokazama 3a0466f
Support `dimmetadata` and `dimmetadatakeys` without `dim` arg
Tokazama 1a418e3
Update src/DataAPI.jl
Tokazama 115962f
Update src/DataAPI.jl
Tokazama 7cb0aef
Improve docs for metadata keys for dims and cols
Tokazama 0d5bb5c
Update src/DataAPI.jl
Tokazama 4a11c37
Update src/DataAPI.jl
Tokazama 8225a7e
Update test/runtests.jl
Tokazama 2ae2768
Update src/DataAPI.jl
Tokazama 57e4f8c
Update src/DataAPI.jl
Tokazama e68ece2
Update src/DataAPI.jl
Tokazama c35f7a1
Update src/DataAPI.jl
Tokazama a3ecbcd
Update src/DataAPI.jl
Tokazama b519d26
Update src/DataAPI.jl
Tokazama ab256ec
Update src/DataAPI.jl
Tokazama 838047a
Update src/DataAPI.jl
Tokazama c0ae557
Update src/DataAPI.jl
Tokazama 2e343f4
Update src/DataAPI.jl
Tokazama b01d2c2
Add doc to sync metadatakeys and metadatasupport
Tokazama 6a0af43
Move `colmetadatasupport` above colmeta functions
Tokazama File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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" | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -17,12 +17,22 @@ 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 | ||||||||||||||
|
||||||||||||||
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] | ||||||||||||||
|
@@ -93,6 +103,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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The docstring says we should throw an error when
Suggested change
|
||||||||||||||
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) | ||||||||||||||
Tokazama marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
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} | ||||||||||||||
|
@@ -301,12 +343,25 @@ end | |||||||||||||
@test_throws MethodError DataAPI.colmetadatakeys(1, 1) | ||||||||||||||
@test_throws MethodError DataAPI.colmetadatakeys(1) | ||||||||||||||
|
||||||||||||||
dim = 1 | ||||||||||||||
Tokazama marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
@test_throws MethodError DataAPI.dimmetadata!(1, dim, "a", 10, style=:default) | ||||||||||||||
@test_throws MethodError DataAPI.deletedimmetadata!(1, dim, "a") | ||||||||||||||
@test_throws MethodError DataAPI.emptydimmetadata!(1, dim) | ||||||||||||||
@test_throws MethodError DataAPI.dimmetadata(1, dim, "a") | ||||||||||||||
@test_throws ArgumentError DataAPI.dimmetadata(1, dim) | ||||||||||||||
@test_throws MethodError DataAPI.dimmetadata(1, dim, "a", style=true) | ||||||||||||||
@test_throws ArgumentError DataAPI.dimmetadata(1, dim, style=true) | ||||||||||||||
@test_throws MethodError DataAPI.dimmetadatakeys(1, dim) | ||||||||||||||
|
||||||||||||||
@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() | ||||||||||||||
|
@@ -353,6 +408,25 @@ end | |||||||||||||
@test DataAPI.colmetadata!(tm, :col, "a", "100", style=:note) == tm | ||||||||||||||
DataAPI.emptycolmetadata!(tm) | ||||||||||||||
@test isempty(DataAPI.colmetadatakeys(tm)) | ||||||||||||||
|
||||||||||||||
@test isempty(DataAPI.dimmetadatakeys(tm, dim)) | ||||||||||||||
@test DataAPI.dimmetadata(tm, dim) == Dict() | ||||||||||||||
@test DataAPI.dimmetadata(tm, dim, style=true) == Dict() | ||||||||||||||
@test DataAPI.dimmetadata!(tm, dim, "a", "100", style=:note) == tm | ||||||||||||||
@test collect(DataAPI.dimmetadatakeys(tm, dim)) == ["a"] | ||||||||||||||
@test_throws KeyError DataAPI.dimmetadata(tm, dim, "b") | ||||||||||||||
@test DataAPI.dimmetadata(tm, dim, "b", 123) == 123 | ||||||||||||||
@test_throws KeyError DataAPI.dimmetadata(tm, dim, "b", style=true) | ||||||||||||||
@test DataAPI.dimmetadata(tm, dim, "b", 123, style=true) == (123, :default) | ||||||||||||||
@test DataAPI.dimmetadata(tm, dim, "a") == "100" | ||||||||||||||
@test DataAPI.dimmetadata(tm, dim) == Dict("a" => "100") | ||||||||||||||
@test DataAPI.dimmetadata(tm, dim, "a", style=true) == ("100", :note) | ||||||||||||||
@test DataAPI.dimmetadata(tm, dim, style=true) == Dict("a" => ("100", :note)) | ||||||||||||||
DataAPI.deletedimmetadata!(tm, dim, "a") | ||||||||||||||
@test isempty(DataAPI.dimmetadatakeys(tm, dim)) | ||||||||||||||
@test DataAPI.dimmetadata!(tm, dim, "a", "100", style=:note) == tm | ||||||||||||||
DataAPI.emptydimmetadata!(tm, dim) | ||||||||||||||
@test isempty(DataAPI.dimmetadatakeys(tm, dim)) | ||||||||||||||
end | ||||||||||||||
|
||||||||||||||
@testset "rownumber" begin | ||||||||||||||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 returnnothing
for dimensions that do not have any metadata.There was a problem hiding this comment.
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?).
There was a problem hiding this comment.
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"? :-)
There was a problem hiding this comment.
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 todimmetadatasupport(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 calldimmetadata(x)
without getting an error? For that you would need to be able to calldimmetadatasupport(T)
.