-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[ntuple] finalize automatic schema evolution between collections #20007
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Collection items must have "_0" as item field name, which can be violated in RVectorField::CreateUntyped().
0c12fd5
to
30e1202
Compare
Test Results 22 files 22 suites 3d 17h 37m 22s ⏱️ Results for commit f420cd5. ♻️ This comment has been updated with latest results. |
30e1202
to
f420cd5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good; two questions inline because I forgot our discussions again...
|
||
void ROOT::RSetField::ReconcileOnDiskField(const RNTupleDescriptor &desc) | ||
{ | ||
static const std::vector<std::string> prefixesRegular = {"std::set<", "std::unordered_set<", "std::map<"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we discussed this, but I forgot the reasoning again, sorry: Why can we read into a kUnorderedSet
from a std::map
, but not from a std::unordered_map
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think from the point of view of I/O we could. But to make it happen, we would need an unordered_set<pair>
, which is missing a default hash function...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from a
std::map
, but not from astd::unordered_map
we would need anunordered_set<pair>
, which is missing a default hash function...
Why is this an issue for a std::unordered_map
?
And is it a fundamental issue or just an issue for the test? I.e. if a user had written a map
and wanted to read it into a unordered_set
they would have have provided the necessary hash overload, isn't it?
|
||
void ROOT::RMapField::ReconcileOnDiskField(const RNTupleDescriptor &desc) | ||
{ | ||
static const std::vector<std::string> prefixesRegular = {"std::map<", "std::unordered_map<", "std::set<"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similarly here: std::unordered_set
?
No description provided.