Adds an abstract type to NamedArrayPartition#447
Adds an abstract type to NamedArrayPartition#447htsnowden wants to merge 2 commits intoSciML:arraypart_zerofrom
Conversation
In order to resolve SciML#443 where I would like to be able to subtype from NamedArrayPartition. This is a large restructuring of NamedArrayPartition but all tests within RecursiveArrayTools do pass. I'm not sure of other tests that I would need to run to ensure further compatibility. This maintains all current functionality but allows a user to create a new subtype of AbstractNamedArrayPartition as long as it maintains the same structure
src/named_array_partition.jl
Outdated
| @@ -1,3 +1,5 @@ | |||
| abstract type AbstractNamedArrayPartition{T, A, NT} <: AbstractVector{T} end | |||
There was a problem hiding this comment.
I think this should have an AbstractArrayPartition, and that needs a docstring to describe its interface?
There was a problem hiding this comment.
Or is ArrayPartition an AbstractNamedArrayPartition with x being its only names?
There was a problem hiding this comment.
I can add a higher level called AbstractArrayPartition{T} if you want and apply the same level of abstraction to the ArrayPartition interface as well (either or both) such that the same subtyping can be performed there also. I will add to the documentation, a docstring about how you can subtype from the AbstractNamedArrayPartition
|
|
||
| function Base.similar(A::NamedArrayPartition) | ||
| NamedArrayPartition( | ||
| # With new type structure this function does the same as Base.similar(x::AbstractNamedArrayPartition{T, S, NT}) where {T, S, NT} |
There was a problem hiding this comment.
Why is this one commented?
There was a problem hiding this comment.
Because I couldn't manage in either the old or the new method to call the less specific function as providing an NamedArrayPartition will have the type structure X{N, A, NT} and therefore a later version would have instead been called. In the original code the two functions are:
function Base.similar(A::NamedArrayPartition)
NamedArrayPartition(
similar(getfield(A, :array_partition)), getfield(A, :names_to_indices))
end
function Base.similar(x::NamedArrayPartition{T, S, NT}) where {T, S, NT}
NamedArrayPartition{T, S, NT}(
similar(ArrayPartition(x)), getfield(x, :names_to_indices))
endBut in all cases I tested the second always overwrites the first as any NamedArrayPartition has the structure of the second
After @ChrisRackauckas comments I have documented how to subtype from AbstractNamedArrayPartition as well as added an AbstractArrayPartition type which both AbstractNamedArrayPartition and ArrayPartition subtype from
|
It's still missing what is meant by the interface. See for example https://github.com/SciML/RecursiveArrayTools.jl/blob/master/src/RecursiveArrayTools.jl#L16-L110. The questions that have to be answered are:
Then we need to make sure the abstract library implementations actually match this. For example, for
|
In order to resolve #443 where I would like to be able to subtype from NamedArrayPartition. This is a large restructuring of NamedArrayPartition but all tests within RecursiveArrayTools do pass. I'm not sure of other tests that I would need to run to ensure further compatibility. This maintains all current functionality but allows a user to create a new subtype of AbstractNamedArrayPartition as long as it maintains the same structure. I am pulling into this branch as this is where my previous fix to the ArrayInterface.zeromatrix lives.
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context