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.

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

materializer behavior differs from its docs #354

Open
aplavin opened this issue Feb 18, 2024 · 0 comments · May be fixed by #355
Open

materializer behavior differs from its docs #354

aplavin opened this issue Feb 18, 2024 · 0 comments · May be fixed by #355

Comments

@aplavin
Copy link

aplavin commented Feb 18, 2024

Docs:

help?> Tables.materializer
  Tables.materializer(x) => Callable

  <...>
  The default materializer is Tables.columntable, which converts any table input into a NamedTuple of Vectors.

  It is recommended that for users implementing MyType, they define only materializer(::Type{<:MyType}). materializer(::MyType) will then
  automatically delegate to this method.

Actual behavior:

julia> using StructArrays, Tables

julia> sa = StructArray(a=[1,2])


# default: returns columntable for the type, ...
julia> Tables.materializer(StructArray)
columntable (generic function with 5 methods)

# ...  but rowtable for an instance
julia> Tables.materializer(sa)
rowtable (generic function with 1 method)


# if I define materializer() for the type, ...
julia> Tables.materializer(::Type{StructArray}) = StructArray  columntable

# ... it works for the type ...
julia> Tables.materializer(StructArray)
StructArray  Tables.columntable

# ... but still returns rowtable for an instance
julia> Tables.materializer(sa)
rowtable (generic function with 1 method)

These are not due to StructArrays doing something weird – it doesn't have materializer definitions yet. I noticed these inconsistencies when adding materializer support there, JuliaArrays/StructArrays.jl#276.

Would be nice to either fix the behavior to match the docs, or fix the docs. I'm not familiar with Tables internals, so it's hard to tell which is correct.

@bkamins bkamins linked a pull request Apr 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant