499 Clean up get_view interface#56
Conversation
Move all AMFFactory invokations to Matrix constructors Add type trait that defines new container types with a possibility to modify a single template parameter. Add vector view type used for gather view on a matrix when one dimension is of size 1 (returning a vector) Remove redundant get_view implementations
| ) {} | ||
|
|
||
| /** | ||
| * Constructor for a view over another storage-based matrix. |
There was a problem hiding this comment.
Should we mark this constructor type as deprecated for now? If it becomes unused over time than we could proceed dropping it. (Comment valid also for other structures)
There was a problem hiding this comment.
We did indeed talk about making it deprecated when it was just delegated to the constructor with IMFs. But then we found another use for it - to make the reshaping views.
There was a problem hiding this comment.
Perhaps we are talking about two different constructors :) I was referring to the AMF-based one below (the first comment line in the code was as close as I could get to the constructor in review). Is this also the one you are referring to?
| base_type( source_vector ) {} | ||
|
|
||
| /** | ||
| * Constructor for a view over another storage-based vector. |
| > | ||
| Vector( TargetType &target_vector, ImfR imf_r, ImfC imf_c ) : | ||
| Vector( SourceType &target_vector, ImfR imf_r, ImfC imf_c ) : | ||
| base_type( getFunctor( target_vector ), imf_r, imf_c ) {} |
There was a problem hiding this comment.
Does Matrix have such constructor?
There was a problem hiding this comment.
I suggest to address this during the clean-up of views on functor-based containers.
| > | ||
| Vector( TargetType &target_vector ) : | ||
| Vector( SourceType &target_vector ) : | ||
| base_type( getFunctor( target_vector ), |
There was a problem hiding this comment.
Does Matrix have such constructor?
|
|
||
| static constexpr Views type_id = Views::gather; | ||
|
|
||
| static std::pair< size_t, size_t > dims( std::pair< size_t, size_t > dims_pair ) { |
There was a problem hiding this comment.
I think we can drop this as it is no longer required by the AMF-based infrastructure.
Also remove the now unnecessary vector view.
|
|
||
| template < bool d > | ||
| struct view_type< view::gather, d > { | ||
| // View -> view::Gather< self_type > ? |
There was a problem hiding this comment.
In what sense? Same comment appears in other specializations.
| ) {} | ||
|
|
||
| /** | ||
| * Constructor for a view over another storage-based matrix. |
There was a problem hiding this comment.
Perhaps we are talking about two different constructors :) I was referring to the AMF-based one below (the first comment line in the code was as close as I could get to the constructor in review). Is this also the one you are referring to?
1 participant
Move all AMFFactory invokations to Matrix constructors
Add type trait that defines new container types with a possibility to modify a single template parameter.
Add vector view type used for gather view on a matrix when one dimension is of size 1 (returning a vector)
Remove redundant get_view implementations