Avoid type piracy via getproperty

[nalimilan]

This method should be replaced with a function owned by Tables.jl:

Tables.jl/src/utils.jl

Line 26 in 7f9a1f9

Base.getproperty(x, ::Type{T}, i::Int, nm::Symbol) where {T} = getproperty(x, nm)

[quinnj]

Maybe.....this is also very, very minor piracy. We're not changing any Base behavior and defining a method signature that isn't likely to collide.

[nalimilan]

Not very likely, but conflicts are not completely impossible either.

[juliohm]

I think that this issue is more general than just type piracy. We were discussing on slack if it would be possible to implement the Tables.jl interface without relying on propertynames nor getproperty. That way we could have a broader class of types implementing the interface like Dict{Symbol,Vector} which are often used as a placeholder for a collection of columns.

Many people were involved in the discussion on slack that I am cc'ing here: @oxinabox, @nalimilan, @bkamins @ararslan. You can check the discussion before slack erases it: https://julialang.slack.com/archives/C6821M4KE/p1578165006066100

Overall, I think there is no strong reason to constrain the interface to propertynames and getproperty. It only has the benefit of allowing the table.foo syntax, which will hardly be used anyways in developer code.

[oxinabox]

the proposal is something like define get_table_field to fall-back to getprooperty,
but allow it to be extended more generally.
E.g.

get_table_field(x, name) = getproperty(x, name)
get_table_field(x::AbstractDict, name) = getindex(x, name)

[juliohm]

One thing I noticed about Dict is that the types of the columns can't be heterogeneous after the dictionary is constructed with a single column type:

d = Dict(:a=>rand(3)) # infers the column types to be Vector{Float64}

d[:b] = rand(Int, 3) # undesirably convert the type from Int to Float64

d[:b] # now contains a Vector{Float64}

Is there any remedy to this issue? It seems severe in this context of tables with multiple column types specified a posteriori.

[nalimilan]

Let's keep this issue focused on the getproperty discussion. Dict issues are separate (I guess you have to create a Dict{Symbol,AbstractVector} explicitly if you want to support columns of different types; when you have homogeneous types, the strict typing is a benefit for type stability so it really depends).

[quinnj]

Ok, I've been mulling this over for a few weeks and while I think I'm on board, and even started implementing it, my one concern is that things may become less convenient for users. While it seems simple to say, "just add get_table_field", that means that for users, to support all inputs, will also have to use get_table_field, instead of being able to rely on getproperty. Maybe that's not that big of a deal, since most "users" are actually packages providing interoperability. Or more specifically, most "users" who care about actually supporting any kind of input table type, will have to use get_table_field, and those will probably be packages for the most part, whereas more casual users will probably be using a specific table where they know the row supports getproperty.

Is there any remedy to this issue? It seems severe in this context of tables with multiple column types specified a posteriori.

This is a non-issue; if you need heterogeneous column types and fully typed columns, use a proper table type, not a Dict. It's just not the right tool for the job. And I don't know of any serious efforts to try and treat Dicts as tables. Not that I'm opposed to adding get_table_field definitions for it, you just have to realize that the dict type will be Dict{Symbol, Any}, unless you can guarantee a homogenous column type.

[nalimilan]

Yes, I admit it wouldn't be as convenient. I guess the question is really: do we want to support Dict as a table type? Other table types probably have no problem with overriding getproperty. Maybe one solution would be to define a simple Dict wrapper which would turn it into a table.

[quinnj]

Fixed in #131