fix fancy eltypes: use ConstructionBase.constructorof

[aplavin]

An example of what gets fixed by this PR is added to runtests.jl: that test would fail before.

[aplavin]

bump...
Tighter integration discussed in the linked issue can be added later if needed at all. This PR is self-contained and seems an improvement (a bugfix even) by itself.

[piever]

I see the issue, but I'm not completely sure the solution is so straightforward. What worries me is that constructorof(T) does not ensure that the output is of type T (whereas createinstance should have that guarantee).

For example,

julia> using ConstructionBase

julia> T = NamedTuple{(:a,), Tuple{Real}};

julia> constructorof(T)(1) isa T
false

This turns out OK here, because T is concrete, but I wonder whether there are more problematic examples. I guess one could assert that the output of createinstance(T, args...) is of type T.

In the long term, I feel like a better fix for the usecase in the testsuite would be to make sure that the type widening is done on the parameters, so that one gets a concrete eltype (eg PS{Int, Union{Int, Nothing}}), but I'm not sure how feasible that is. There would need to be a "container-dependent" way to widen type in Julia Base map and broadcast machinery.

[aplavin]

What worries me is that constructorof(T) does not ensure that the output is of type T (whereas createinstance should have that guarantee).

I believe there were discussions about asserting that constructors always return the requested type T(...)::T in Julia, but it's not the case now. Constructors can return whatever type they want, although this indeed happens rarer than with constructorof.

make sure that the type widening is done on the parameters, so that one gets a concrete eltype (eg PS{Int, Union{Int, Nothing}}), but I'm not sure how feasible that is

This would probably change the return type of regular map and maybe other functions, so unlikely to happen in Julia 1.

[aplavin]

Bump... I see this PR as an unambiguous improvement, even though not perfect (ie doesn't get rid of the whole create_instance function).

Technically neither regular constructor T(...) nor constructorof(T)(...) guarantee that they return the T type. In practice, I would say the constructorof approach is more reliable: its docs say

It must be possible to reconstruct an object from the elements of getfields:
ctor = constructorof(typeof(obj))
@Assert obj == ctor(getfields(obj)...)
@Assert typeof(obj) == typeof(ctor(getfields(obj)...))

Meanwhile, regular T(...) constructors can do whatever processing they want and take arbitrary arguments – not necessarily fields of the type.

One doesn't even need to go outside of Base types to break StructArrays. This:

StructArray(LinRange[
	LinRange(1, 2, 3),
	LinRange(1f0, 2f0, 3),
])

also doesn't work now, same as the example in this PR tests. But would work with constructorof().

[aplavin]

Bump...
@piever

[aplavin]

Another bump

[piever]

Sorry for the very slow reaction time. I confess I'm hesitant to make fundamental changes such as this one to StructArrays (esp. since I have very limited time to dedicate to the package, so I wouldn't really be able to address any issues that could stem from this).

OTOH, this does seem like a practical improvement, and the most important case of concrete element type is preserved, so it should be fine. I'll ask on the arrays Slack if there are strong opinions either way, otherwise I imagine it can be merged.

[aplavin]

bump!

[piever]

Rebased and merged in #280