similar should not pirate Array Types in methods?

[vtjnash]

It seems like the only reason similar would be called on Array{<:CategoricalValue} with a new eltype of CategoricalValue means the user apparently explicitly wanted an Array (otherwise, it would have been called with an AbstractRange typeof(1:1) for instance). It seems like this package might be not following user expectations here, which is also appearing to cause many invalidations during loading, leading to worse precompile results across the Julia ecosystem. I think we should delete these extra similar methods, so that requests for a similar object to a Base Vector does indeed return some similar Vector to the original. But keep the methods for typeof(1:1), so that collect with a CategoricalValue does still get the CategoricalArray as intended, when the user has not otherwise tried to explicitly ask for it to return a Base.Vector type as the argument to collect_to?

[nalimilan]

I've just tried this, but it seems difficult to completely get rid of these definitions. We still want e.g. similar(::Array{Int}, ::CategoricalValue) to return a CategoricalArray, as this pattern is generally used when you want an array of the most appropriate type for a given eltype. We do this a lot in DataFrames for example. In the vast majority of cases, users want a CategoricalArray and would be very confused to get an Array{<:CategoricalValue}, which doesn't work in all situations where CategoricalArray does and is much less efficient.

[bkamins]

Do I understand correctly we refer to this signature:

similar(array, [element_type=eltype(array)], [dims=size(array)])

(i.e. similar with array passed as an instance of an array).

In this case the docstrings say that:

Custom AbstractArray subtypes may choose which specific array type is best-suited to return for the given element type and
  dimensionality. If they do not specialize this method, the default is an Array{element_type}(undef, dims...).

So I guess, contract-wise, this should be acceptable to return CategoricalArray in cases when user wants CategoricalValue as eltype.
I understand the invalidations issue, but @nalimilan - if I understand you correctly, the pattern using similar to create CategoricalArray is used in the ecosystem.

[nalimilan]

What diverges somewhat from the docstring is that we override methods where the first argument is an Array rather than a CategoricalArray. But as I said it seems that in many cases this is what people want. If you absolutely want an Array you can do Array{eltype(x)}(undef, size(x)).