Prepare "immer vs classic" demo.#7
Conversation
|
Thanks! I'm in! |
jcferretti
left a comment
jcferretti
left a comment
There was a problem hiding this comment.
A kind of general comment... seems like the Context arguments to fillChunk and friends are never used? Should we just get rid of them, or do they have some future intended value?
|
|
||
| void fillChunkUnordered(Context *context, const UInt64Chunk &rowKeys, Chunk *dest) const final { | ||
| using deephaven::client::utility::stringf; | ||
| throw std::runtime_error(stringf("TODO(kosak): %o", DEEPHAVEN_PRETTY_FUNCTION)); |
There was a problem hiding this comment.
Is this TODO safe for us to merge/move on to next task?
| }; | ||
|
|
||
| template<typename T> | ||
| void ImmerColumnSource<T>::fillChunk(Context *context, const RowSequence &rows, Chunk *dest) const { |
There was a problem hiding this comment.
Context is unused here, should we remove the name?
There was a problem hiding this comment.
We discussed on VC, we agreed to remove Context.
| @@ -2,16 +2,13 @@ | |||
|
|
|||
| #include <memory> | |||
| #include <vector> | |||
| #include "deephaven/client/highlevel/columns.h" | |||
There was a problem hiding this comment.
I just noticed that you got rid of "highlevel" etc; care to elaborate a bit?
There was a problem hiding this comment.
In the previous version of the C++ library (the one that spoke the GWT protocol to Deephaven Enterprise) there was a pretty clear distinction between "low level" and "high level" interactions. In low-level interactions you could issue server commands yourself (various GWT calls... where I interpret GWT as the precursor to GRPC). In high-level interactions you would get your data wrapped up in ColumnSources, have Table abstractions, have automatic lazy evaluation, and also you would get all stuff in the "fluent" layer (the operator overloading stuff). The highlevel stuff was a layer on top of the lowlevel stuff.
For this version I decided that we weren't really getting any value out of the distinction between highlevel vs lowlevel. For example, we're not offering our users the ability to send their own GRPC messages to the server via our library. So for these reasons I ripped it out.
|
From the point of view of considering this "user ready" I think we are going to need to make a decision in terms of if we want to devote some amount of effort to build a minimal unit test suite (I think at this point we don't have any unit tests; in terms of testing (understood as exercising the code), we only have the small examples/driver code we have been using to experiment/measure, correct?). A possible way to go here would be to say "we want to invest X days building unit tests", and then use that time to build, value for effort, tests for the things that would add more value. We should discuss. |
2 participants
Putting this in a PR for Cristian to review