[WIP] Use DataAPI.metadata instead of redefining it

[asinghvi17]

Fix #706 by explicitly importing DataAPI.metadata.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.87%. Comparing base (37cd1eb) to head (05c464d). Report is 3 commits behind head on main.

:exclamation: Current head 05c464d differs from pull request most recent head a10403b. Consider uploading reports for the commit a10403b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #707      +/-   ##
==========================================
+ Coverage   83.86%   83.87%   +0.01%     
==========================================
  Files          46       46              
  Lines        4257     4267      +10     
==========================================
+ Hits         3570     3579       +9     
- Misses        687      688       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[rafaqz]

May need to check we don't have function metadata end or metadata(x) = nothing lying around

[rafaqz]

We have function metadata end in interface.jl that needs removing.

And these methods are going to be a piracy problem:

  [2] metadata(::Tuple{})
     @ DimensionalData.Dimensions ~/.julia/dev/DimensionalData/src/Dimensions/dimension.jl:232
 [15] metadata(ds::Tuple, I)
     @ DimensionalData.Dimensions ~/.julia/dev/DimensionalData/src/Dimensions/dimension.jl:234
 [16] metadata(ds::Tuple, i1, I...)
     @ DimensionalData.Dimensions ~/.julia/dev/DimensionalData/src/Dimensions/dimension.jl:233
 [17] metadata(ds::Tuple)
     @ DimensionalData.Dimensions ~/.julia/dev/DimensionalData/src/Dimensions/dimension.jl:231
 [18] metadata(x)
     @ ~/.julia/dev/DimensionalData/src/Lookups/metadata.jl:107

I guess we can make all the Tuples into DimTuple so we own them, with the slight downside that it will error on empty Tuple{}.

The untyped case may be a problem too, we can try deleting it and see what breaks.

[asinghvi17]

Thanks - will take a look today!

[asinghvi17]

julia> DataAPI.metadata(tuple())
ERROR: ArgumentError: Objects of type Tuple{} do not support reading metadata
Stacktrace:
 [1] metadata(x::Tuple{}; style::Bool)
   @ DataAPI ~/.julia/packages/DataAPI/atdEM/src/DataAPI.jl:373
 [2] metadata(x::Tuple{})
   @ DataAPI ~/.julia/packages/DataAPI/atdEM/src/DataAPI.jl:371
 [3] top-level scope
   @ REPL[41]:1

so that method is at least open...

[asinghvi17]

Ah damn, I just realized that the way that DD handles metadata (DD.NoLookup) is not the same way as DataAPI handles metadata (Nothing). I guess this could potentially stay the same for now, and maybe just hook into DataAPI.metadata as well for DD native types...

[rafaqz]

DD.NoMetadata but we could maybe change it. One nice thing about NoMetadata is Dict functions like haskey work on it (and we can as more as needed) so its a nice fallback that both wont cause errors anywhere and e.g. works on GPUs like nothing would.

[rafaqz]

From discussion: we could instead forward DataAPI.metadata to DimensionalData.metadata so at least they both work.

[rafaqz]

Actually thinking about just culling all the DD metadata handling so we use a Dict or NamedTuple or nothing.

It will be annoying to check keys but its worth it to use DataAPI.metadata