Added vcat for ColVecs/RowVecs #291
Conversation
|
Can you also add some tests? |
Co-authored-by: David Widmann <[email protected]>
There was a problem hiding this comment.
LGTM. Just requires a patch version bump then we're good to go provided that @devmotion is happy.
There was a problem hiding this comment.
I think the tests could be improved a bit and actually check that we do the correct thing, e.g., by concatenating two different objects and checking that the concatenated values are correct (I also don't think that the check typeof(DX) == typeof(vcat(DX, DX)) is important - it's not the main purpose of the implementation and only a by-product if one actually concatenates two RowVecs or ColVecs whose underlying matrices have the same type). Otherwise, only the version bump seems to be missing 🙂
|
@devmotion I've modified the tests to what you suggested - nothing extensive, but given the simplicity of the PR I wasn't at my most creative. Let me know if you have other suggestions! |
Co-authored-by: David Widmann <[email protected]>
|
Thanks a lot for the PR 👍 |
Tentative fix for this issue in AbstractGPs.