Add tests for first C++ types #36
Conversation
| RNTupleWriter::Recreate(std::move(model), "ntpl", filename, options); | ||
|
|
||
| // First entry: std::int32_t | ||
| *value = 1; |
There was a problem hiding this comment.
Should we also test the case of Fill without a value set?
There was a problem hiding this comment.
IIRC a default-constructed std::variant has a default-constructed value of its first alternative, so std::int32_t{0} in this case. For me, that's more of a C++ detail than specific to RNTuple.
What we are really missing is valueless_by_exception, but for that we need a type to throw an exception during move assignment (see https://en.cppreference.com/w/cpp/utility/variant/valueless_by_exception). I added a note to README.md.
|
|
||
| ## Entries | ||
|
|
||
| 1. One character strings, with ascending values |
There was a problem hiding this comment.
General comment/question. Was the README intended to be sufficient to know what is in the file? As is the README(s) are general description and knowing what is really in the file require parsing/reading the write.C (or later the reference file). Is that the intent?
There was a problem hiding this comment.
That's a good question and I haven't fully made up my mind yet. I think I see arguments for both ways (basically a full standalone description vs duplication in multiple places), so for now I was lazy and decided not to duplicate. If we decide that we want a full description in the READMEs, I can put the following structure (in a follow-up PR):
- One character strings, with ascending values:
Index32:"a"Index64:"b"SplitIndex32:"c"SplitIndex64:"d"
but it would literally by the same as in the write.C...
... with all possible index column types. Closes root-project#2
... with all possible index column types. Closes root-project#3
std::string(closes Test forstd::stringtype #2)std::vector(closes Tests forstd::vectortype #3)std::variant(closes Test forstd::varianttype #4)