From c0d6df63a637d35eecc214b08af62c24b0619e6b Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Sun, 20 Oct 2024 11:35:46 +0200 Subject: [PATCH 1/2] [ntuple] Rename RNTupleModel::GetField to GetConstField --- tree/ntuple/v7/inc/ROOT/RNTupleModel.hxx | 2 +- tree/ntuple/v7/src/RNTupleModel.cxx | 4 +- tree/ntuple/v7/src/RPageSinkBuf.cxx | 5 ++- tree/ntuple/v7/test/ntuple_compat.cxx | 14 +++--- tree/ntuple/v7/test/ntuple_model.cxx | 22 +++++----- tree/ntuple/v7/test/ntuple_modelext.cxx | 14 +++--- tree/ntuple/v7/test/ntuple_multi_column.cxx | 38 ++++++++-------- .../ntuple/v7/test/ntuple_parallel_writer.cxx | 6 +-- tree/ntuple/v7/test/ntuple_storage.cxx | 4 +- tree/ntuple/v7/test/ntuple_types.cxx | 44 ++++++++++--------- tree/ntuple/v7/test/rfield_class.cxx | 2 +- tree/ntupleutil/v7/test/ntuple_importer.cxx | 4 +- tree/ntupleutil/v7/test/ntuple_inspector.cxx | 2 +- 13 files changed, 82 insertions(+), 79 deletions(-) diff --git a/tree/ntuple/v7/inc/ROOT/RNTupleModel.hxx b/tree/ntuple/v7/inc/ROOT/RNTupleModel.hxx index 181ffee2939eb..105d3b12379fb 100644 --- a/tree/ntuple/v7/inc/ROOT/RNTupleModel.hxx +++ b/tree/ntuple/v7/inc/ROOT/RNTupleModel.hxx @@ -363,7 +363,7 @@ public: /// Mutable access to the root field is used to make adjustments to the fields. RFieldZero &GetMutableFieldZero(); const RFieldZero &GetConstFieldZero() const { return *fFieldZero; } - const RFieldBase &GetField(std::string_view fieldName) const; + const RFieldBase &GetConstField(std::string_view fieldName) const; const std::string &GetDescription() const { return fDescription; } void SetDescription(std::string_view description); diff --git a/tree/ntuple/v7/src/RNTupleModel.cxx b/tree/ntuple/v7/src/RNTupleModel.cxx index f3477957880cc..902d5ca1d359b 100644 --- a/tree/ntuple/v7/src/RNTupleModel.cxx +++ b/tree/ntuple/v7/src/RNTupleModel.cxx @@ -155,7 +155,7 @@ ROOT::Experimental::Internal::RProjectedFields::Clone(const RNTupleModel &newMod for (const auto &[k, v] : fFieldMap) { for (const auto &f : clone->GetFieldZero()) { if (f.GetQualifiedFieldName() == k->GetQualifiedFieldName()) { - clone->fFieldMap[&f] = &newModel.GetField(v->GetQualifiedFieldName()); + clone->fFieldMap[&f] = &newModel.GetConstField(v->GetQualifiedFieldName()); break; } } @@ -354,7 +354,7 @@ ROOT::Experimental::RFieldZero &ROOT::Experimental::RNTupleModel::GetMutableFiel return *fFieldZero; } -const ROOT::Experimental::RFieldBase &ROOT::Experimental::RNTupleModel::GetField(std::string_view fieldName) const +const ROOT::Experimental::RFieldBase &ROOT::Experimental::RNTupleModel::GetConstField(std::string_view fieldName) const { auto f = FindField(fieldName); if (!f) diff --git a/tree/ntuple/v7/src/RPageSinkBuf.cxx b/tree/ntuple/v7/src/RPageSinkBuf.cxx index fb3771f999eb6..7a1d69dc4d24f 100644 --- a/tree/ntuple/v7/src/RPageSinkBuf.cxx +++ b/tree/ntuple/v7/src/RPageSinkBuf.cxx @@ -107,10 +107,11 @@ void ROOT::Experimental::Internal::RPageSinkBuf::UpdateSchema(const RNTupleModel auto p = &(*cloned); auto &projectedFields = Internal::GetProjectedFieldsOfModel(changeset.fModel); Internal::RProjectedFields::FieldMap_t fieldMap; - fieldMap[p] = &fInnerModel->GetField(projectedFields.GetSourceField(field)->GetQualifiedFieldName()); + fieldMap[p] = &fInnerModel->GetConstField(projectedFields.GetSourceField(field)->GetQualifiedFieldName()); auto targetIt = cloned->begin(); for (auto &f : *field) - fieldMap[&(*targetIt++)] = &fInnerModel->GetField(projectedFields.GetSourceField(&f)->GetQualifiedFieldName()); + fieldMap[&(*targetIt++)] = + &fInnerModel->GetConstField(projectedFields.GetSourceField(&f)->GetQualifiedFieldName()); Internal::GetProjectedFieldsOfModel(*fInnerModel).Add(std::move(cloned), fieldMap); return p; }; diff --git a/tree/ntuple/v7/test/ntuple_compat.cxx b/tree/ntuple/v7/test/ntuple_compat.cxx index b6254e5080a55..36e36dfe1fa6a 100644 --- a/tree/ntuple/v7/test/ntuple_compat.cxx +++ b/tree/ntuple/v7/test/ntuple_compat.cxx @@ -284,9 +284,9 @@ TEST(RNTupleCompat, FutureColumnType) auto model = desc.CreateModel(modelOpts); // The future column should not show up in the model - EXPECT_THROW(model->GetField("futureColumn"), RException); + EXPECT_THROW(model->GetConstField("futureColumn"), RException); - const auto &floatFld = model->GetField("float"); + const auto &floatFld = model->GetConstField("float"); EXPECT_EQ(floatFld.GetTypeName(), "float"); reader.reset(); @@ -336,9 +336,9 @@ TEST(RNTupleCompat, FutureColumnType_Nested) auto model = desc.CreateModel(modelOpts); // The future column should not show up in the model - EXPECT_THROW(model->GetField("future"), RException); + EXPECT_THROW(model->GetConstField("future"), RException); - const auto &floatFld = model->GetField("float"); + const auto &floatFld = model->GetConstField("float"); EXPECT_EQ(floatFld.GetTypeName(), "float"); reader.reset(); @@ -395,7 +395,7 @@ TEST(RNTupleCompat, FutureFieldStructuralRole) modelOpts.fForwardCompatible = true; auto model = desc.CreateModel(modelOpts); try { - model->GetField("future"); + model->GetConstField("future"); FAIL() << "trying to get a field with unknown role should fail"; } catch (const RException &err) { EXPECT_THAT(err.what(), testing::HasSubstr("invalid field")); @@ -431,10 +431,10 @@ TEST(RNTupleCompat, FutureFieldStructuralRole_Nested) auto modelOpts = RNTupleDescriptor::RCreateModelOptions(); modelOpts.fForwardCompatible = true; auto model = desc.CreateModel(modelOpts); - const auto &floatFld = model->GetField("float"); + const auto &floatFld = model->GetConstField("float"); EXPECT_EQ(floatFld.GetTypeName(), "float"); try { - model->GetField("record"); + model->GetConstField("record"); FAIL() << "trying to get a field with unknown role should fail"; } catch (const RException &err) { EXPECT_THAT(err.what(), testing::HasSubstr("invalid field")); diff --git a/tree/ntuple/v7/test/ntuple_model.cxx b/tree/ntuple/v7/test/ntuple_model.cxx index 4ae7986ca7d1f..507f7697363d9 100644 --- a/tree/ntuple/v7/test/ntuple_model.cxx +++ b/tree/ntuple/v7/test/ntuple_model.cxx @@ -68,18 +68,18 @@ TEST(RNTupleModel, GetField) m->MakeField("x"); m->MakeField("cs"); m->Freeze(); - EXPECT_EQ(m->GetField("x").GetFieldName(), "x"); - EXPECT_EQ(m->GetField("x").GetTypeName(), "std::int32_t"); - EXPECT_EQ(m->GetField("cs.v1").GetFieldName(), "v1"); - EXPECT_EQ(m->GetField("cs.v1").GetTypeName(), "std::vector"); + EXPECT_EQ(m->GetConstField("x").GetFieldName(), "x"); + EXPECT_EQ(m->GetConstField("x").GetTypeName(), "std::int32_t"); + EXPECT_EQ(m->GetConstField("cs.v1").GetFieldName(), "v1"); + EXPECT_EQ(m->GetConstField("cs.v1").GetTypeName(), "std::vector"); try { - m->GetField("nonexistent"); + m->GetConstField("nonexistent"); FAIL() << "invalid field name should throw"; } catch (const RException &err) { EXPECT_THAT(err.what(), testing::HasSubstr("invalid field")); } try { - m->GetField(""); + m->GetConstField(""); FAIL() << "empty field name should throw"; } catch (const RException &err) { EXPECT_THAT(err.what(), testing::HasSubstr("invalid field")); @@ -91,9 +91,9 @@ TEST(RNTupleModel, GetField) EXPECT_THAT(err.what(), testing::HasSubstr("frozen model")); } EXPECT_EQ("", m->GetConstFieldZero().GetQualifiedFieldName()); - EXPECT_EQ("x", m->GetField("x").GetQualifiedFieldName()); - EXPECT_EQ("cs", m->GetField("cs").GetQualifiedFieldName()); - EXPECT_EQ("cs.v1", m->GetField("cs.v1").GetQualifiedFieldName()); + EXPECT_EQ("x", m->GetConstField("x").GetQualifiedFieldName()); + EXPECT_EQ("cs", m->GetConstField("cs").GetQualifiedFieldName()); + EXPECT_EQ("cs.v1", m->GetConstField("cs.v1").GetQualifiedFieldName()); } TEST(RNTupleModel, EstimateWriteMemoryUsage) @@ -160,6 +160,6 @@ TEST(RNTupleModel, Clone) EXPECT_EQ(EColumnType::kUInt32, f.GetColumnRepresentatives()[0][0]); } } - EXPECT_TRUE(clone->GetField("struct").GetTraits() & RFieldBase::kTraitTypeChecksum); - EXPECT_TRUE(clone->GetField("obj").GetTraits() & RFieldBase::kTraitTypeChecksum); + EXPECT_TRUE(clone->GetConstField("struct").GetTraits() & RFieldBase::kTraitTypeChecksum); + EXPECT_TRUE(clone->GetConstField("obj").GetTraits() & RFieldBase::kTraitTypeChecksum); } diff --git a/tree/ntuple/v7/test/ntuple_modelext.cxx b/tree/ntuple/v7/test/ntuple_modelext.cxx index 3477726eeb95f..bcaa0456db36d 100644 --- a/tree/ntuple/v7/test/ntuple_modelext.cxx +++ b/tree/ntuple/v7/test/ntuple_modelext.cxx @@ -61,8 +61,8 @@ TEST(RNTuple, ModelExtensionSimple) auto ntuple = RNTupleReader::Open("myNTuple", fileGuard.GetPath()); EXPECT_EQ(4U, ntuple->GetNEntries()); EXPECT_EQ(4U, ntuple->GetDescriptor().GetNFields()); - EXPECT_EQ(0U, GetFirstEntry(ntuple->GetModel().GetField("pt"))); - EXPECT_EQ(2U, GetFirstEntry(ntuple->GetModel().GetField("array"))); + EXPECT_EQ(0U, GetFirstEntry(ntuple->GetModel().GetConstField("pt"))); + EXPECT_EQ(2U, GetFirstEntry(ntuple->GetModel().GetConstField("array"))); auto pt = ntuple->GetView("pt"); auto array = ntuple->GetView>("array"); @@ -155,11 +155,11 @@ TEST(RNTuple, ModelExtensionMultiple) auto ntuple = RNTupleReader::Open("myNTuple", fileGuard.GetPath()); EXPECT_EQ(5U, ntuple->GetNEntries()); EXPECT_EQ(7U, ntuple->GetDescriptor().GetNFields()); - EXPECT_EQ(0U, GetFirstEntry(ntuple->GetModel().GetField("pt"))); - EXPECT_EQ(3U, GetFirstEntry(ntuple->GetModel().GetField("vec"))); - EXPECT_EQ(3U, GetFirstEntry(ntuple->GetModel().GetField("f"))); - EXPECT_EQ(3U, GetFirstEntry(ntuple->GetModel().GetField("u8"))); - EXPECT_EQ(3U, GetFirstEntry(ntuple->GetModel().GetField("str"))); + EXPECT_EQ(0U, GetFirstEntry(ntuple->GetModel().GetConstField("pt"))); + EXPECT_EQ(3U, GetFirstEntry(ntuple->GetModel().GetConstField("vec"))); + EXPECT_EQ(3U, GetFirstEntry(ntuple->GetModel().GetConstField("f"))); + EXPECT_EQ(3U, GetFirstEntry(ntuple->GetModel().GetConstField("u8"))); + EXPECT_EQ(3U, GetFirstEntry(ntuple->GetModel().GetConstField("str"))); auto pt = ntuple->GetView("pt"); auto vec = ntuple->GetView>("vec"); diff --git a/tree/ntuple/v7/test/ntuple_multi_column.cxx b/tree/ntuple/v7/test/ntuple_multi_column.cxx index ec44bdb9bb39c..aa8afb4fdab0c 100644 --- a/tree/ntuple/v7/test/ntuple_multi_column.cxx +++ b/tree/ntuple/v7/test/ntuple_multi_column.cxx @@ -18,18 +18,18 @@ TEST(RNTuple, MultiColumnRepresentationSimple) writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("px")), 1); + const_cast(writer->GetModel().GetConstField("px")), 1); *ptrPx = 2.0; writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("px")), 0); + const_cast(writer->GetModel().GetConstField("px")), 0); *ptrPx = 3.0; writer->Fill(); } auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); - EXPECT_EQ(3u, reader->GetModel().GetField("px").GetNElements()); + EXPECT_EQ(3u, reader->GetModel().GetConstField("px").GetNElements()); const auto &desc = reader->GetDescriptor(); EXPECT_EQ(3u, desc.GetNClusters()); @@ -157,16 +157,16 @@ TEST(RNTuple, MultiColumnRepresentationString) writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("str")), 1); + const_cast(writer->GetModel().GetConstField("str")), 1); ptrStr->clear(); writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("str")), 0); + const_cast(writer->GetModel().GetConstField("str")), 0); writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("str")), 1); + const_cast(writer->GetModel().GetConstField("str")), 1); *ptrStr = "x"; writer->Fill(); } @@ -199,17 +199,17 @@ TEST(RNTuple, MultiColumnRepresentationVector) writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("vec")), 1); + const_cast(writer->GetModel().GetConstField("vec")), 1); ptrVec->push_back(1.0); writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("vec")), 0); + const_cast(writer->GetModel().GetConstField("vec")), 0); ptrVec->clear(); writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("vec")), 1); + const_cast(writer->GetModel().GetConstField("vec")), 1); ptrVec->push_back(2.0); ptrVec->push_back(3.0); writer->Fill(); @@ -250,17 +250,17 @@ TEST(RNTuple, MultiColumnRepresentationMany) writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("vec")), 1); + const_cast(writer->GetModel().GetConstField("vec")), 1); (*ptrVec)[0] = 2.0; writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("vec")), 2); + const_cast(writer->GetModel().GetConstField("vec")), 2); (*ptrVec)[0] = 3.0; writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("vec")), 3); + const_cast(writer->GetModel().GetConstField("vec")), 3); (*ptrVec)[0] = 4.0; writer->Fill(); } @@ -302,18 +302,18 @@ TEST(RNTuple, MultiColumnRepresentationNullable) writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("scalar")), 1); + const_cast(writer->GetModel().GetConstField("scalar")), 1); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("vector._0")), 1); + const_cast(writer->GetModel().GetConstField("vector._0")), 1); ptrScalar->reset(); ptrVector->clear(); ptrVector->push_back(std::optional()); writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("scalar")), 0); + const_cast(writer->GetModel().GetConstField("scalar")), 0); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("vector._0")), 0); + const_cast(writer->GetModel().GetConstField("vector._0")), 0); *ptrScalar = 3.0; ptrVector->clear(); ptrVector->push_back(15.0); @@ -362,7 +362,7 @@ TEST(RNTuple, MultiColumnRepresentationBulk) writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("px")), 1); + const_cast(writer->GetModel().GetConstField("px")), 1); *ptrPx = 2.0; writer->Fill(); } @@ -402,7 +402,7 @@ TEST(RNTuple, MultiColumnRepresentationFriends) writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("pt")), 1); + const_cast(writer->GetModel().GetConstField("pt")), 1); *ptrPt = 2.0; writer->Fill(); } @@ -412,7 +412,7 @@ TEST(RNTuple, MultiColumnRepresentationFriends) writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("eta")), 1); + const_cast(writer->GetModel().GetConstField("eta")), 1); *ptrEta = 4.0; writer->Fill(); } diff --git a/tree/ntuple/v7/test/ntuple_parallel_writer.cxx b/tree/ntuple/v7/test/ntuple_parallel_writer.cxx index 4220a20a286a8..1954a7b8d1404 100644 --- a/tree/ntuple/v7/test/ntuple_parallel_writer.cxx +++ b/tree/ntuple/v7/test/ntuple_parallel_writer.cxx @@ -179,12 +179,12 @@ TEST(RNTupleParallelWriter, StagedMultiColumn) c->Fill(*e); c->FlushCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(c->GetModel().GetField("px")), 1); + const_cast(c->GetModel().GetConstField("px")), 1); *px = 2.0; c->Fill(*e); c->FlushCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(c->GetModel().GetField("px")), 0); + const_cast(c->GetModel().GetConstField("px")), 0); *px = 3.0; c->Fill(*e); c->FlushCluster(); @@ -193,7 +193,7 @@ TEST(RNTupleParallelWriter, StagedMultiColumn) } auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); - EXPECT_EQ(3u, reader->GetModel().GetField("px").GetNElements()); + EXPECT_EQ(3u, reader->GetModel().GetConstField("px").GetNElements()); const auto &desc = reader->GetDescriptor(); EXPECT_EQ(3u, desc.GetNClusters()); diff --git a/tree/ntuple/v7/test/ntuple_storage.cxx b/tree/ntuple/v7/test/ntuple_storage.cxx index 5ac45040b03ca..58d67604ff5b5 100644 --- a/tree/ntuple/v7/test/ntuple_storage.cxx +++ b/tree/ntuple/v7/test/ntuple_storage.cxx @@ -864,8 +864,8 @@ TEST(RPageStorageFile, MultiKeyBlob_Header) EXPECT_GE(ntupleUcmp->GetDescriptor().GetOnDiskHeaderSize(), kMaxKeySize); for (int i = 0; i < kNFields; ++i) { - ntupleComp->GetModel().GetField(std::to_string(i)); - ntupleUcmp->GetModel().GetField(std::to_string(i)); + ntupleComp->GetModel().GetConstField(std::to_string(i)); + ntupleUcmp->GetModel().GetConstField(std::to_string(i)); } } } diff --git a/tree/ntuple/v7/test/ntuple_types.cxx b/tree/ntuple/v7/test/ntuple_types.cxx index 5ada937d3a028..83b98bb619107 100644 --- a/tree/ntuple/v7/test/ntuple_types.cxx +++ b/tree/ntuple/v7/test/ntuple_types.cxx @@ -42,7 +42,7 @@ TEST(RNTuple, EnumBasics) auto ptrVecEnum = model->MakeField>("ve"); model->MakeField("swe"); - EXPECT_EQ(model->GetField("e").GetTypeName(), f->GetTypeName()); + EXPECT_EQ(model->GetConstField("e").GetTypeName(), f->GetTypeName()); FileRaii fileGuard("test_ntuple_enum_basics.root"); { @@ -1397,8 +1397,8 @@ TEST(RNTuple, Bitset) auto model = RNTupleModel::Create(); auto f1 = model->MakeField>("f1"); - EXPECT_EQ(std::string("std::bitset<66>"), model->GetField("f1").GetTypeName()); - EXPECT_EQ(sizeof(std::bitset<66>), model->GetField("f1").GetValueSize()); + EXPECT_EQ(std::string("std::bitset<66>"), model->GetConstField("f1").GetTypeName()); + EXPECT_EQ(sizeof(std::bitset<66>), model->GetConstField("f1").GetValueSize()); auto f2 = model->MakeField>("f2", "10101010"); { @@ -1413,7 +1413,7 @@ TEST(RNTuple, Bitset) } auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath()); - EXPECT_EQ(std::string("std::bitset<66>"), reader->GetModel().GetField("f1").GetTypeName()); + EXPECT_EQ(std::string("std::bitset<66>"), reader->GetModel().GetConstField("f1").GetTypeName()); auto bs1 = reader->GetModel().GetDefaultEntry().GetPtr>("f1"); auto bs2 = reader->GetModel().GetDefaultEntry().GetPtr>("f2"); reader->LoadEntry(0); @@ -1437,12 +1437,12 @@ TEST(RNTuple, UniquePtr) auto ppString = model->MakeField>>("PPString"); auto pArray = model->MakeField>>("PArray"); - EXPECT_EQ("std::unique_ptr", model->GetField("PBool").GetTypeName()); - EXPECT_EQ(std::string("std::unique_ptr"), model->GetField("PCustomStruct").GetTypeName()); - EXPECT_EQ(std::string("std::unique_ptr"), model->GetField("PIOConstructor").GetTypeName()); + EXPECT_EQ("std::unique_ptr", model->GetConstField("PBool").GetTypeName()); + EXPECT_EQ(std::string("std::unique_ptr"), model->GetConstField("PCustomStruct").GetTypeName()); + EXPECT_EQ(std::string("std::unique_ptr"), model->GetConstField("PIOConstructor").GetTypeName()); EXPECT_EQ(std::string("std::unique_ptr>"), - model->GetField("PPString").GetTypeName()); - EXPECT_EQ(std::string("std::unique_ptr>"), model->GetField("PArray").GetTypeName()); + model->GetConstField("PPString").GetTypeName()); + EXPECT_EQ(std::string("std::unique_ptr>"), model->GetConstField("PArray").GetTypeName()); auto writer = RNTupleWriter::Recreate(std::move(model), "ntuple", fileGuard.GetPath()); @@ -1476,11 +1476,12 @@ TEST(RNTuple, UniquePtr) auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath()); const auto &model = reader->GetModel(); - EXPECT_EQ("std::unique_ptr", model.GetField("PBool").GetTypeName()); - EXPECT_EQ(std::string("std::unique_ptr"), model.GetField("PCustomStruct").GetTypeName()); - EXPECT_EQ(std::string("std::unique_ptr"), model.GetField("PIOConstructor").GetTypeName()); - EXPECT_EQ(std::string("std::unique_ptr>"), model.GetField("PPString").GetTypeName()); - EXPECT_EQ(std::string("std::unique_ptr>"), model.GetField("PArray").GetTypeName()); + EXPECT_EQ("std::unique_ptr", model.GetConstField("PBool").GetTypeName()); + EXPECT_EQ(std::string("std::unique_ptr"), model.GetConstField("PCustomStruct").GetTypeName()); + EXPECT_EQ(std::string("std::unique_ptr"), model.GetConstField("PIOConstructor").GetTypeName()); + EXPECT_EQ(std::string("std::unique_ptr>"), + model.GetConstField("PPString").GetTypeName()); + EXPECT_EQ(std::string("std::unique_ptr>"), model.GetConstField("PArray").GetTypeName()); const auto &entry = model.GetDefaultEntry(); auto pBool = entry.GetPtr>("PBool"); @@ -1820,10 +1821,10 @@ TEST(RNTuple, Double32) } auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath()); - EXPECT_EQ(EColumnType::kReal32, reader->GetModel().GetField("d1").GetColumnRepresentatives()[0][0]); - EXPECT_EQ("", reader->GetModel().GetField("d1").GetTypeAlias()); - EXPECT_EQ(EColumnType::kSplitReal32, reader->GetModel().GetField("d2").GetColumnRepresentatives()[0][0]); - EXPECT_EQ("Double32_t", reader->GetModel().GetField("d2").GetTypeAlias()); + EXPECT_EQ(EColumnType::kReal32, reader->GetModel().GetConstField("d1").GetColumnRepresentatives()[0][0]); + EXPECT_EQ("", reader->GetModel().GetConstField("d1").GetTypeAlias()); + EXPECT_EQ(EColumnType::kSplitReal32, reader->GetModel().GetConstField("d2").GetColumnRepresentatives()[0][0]); + EXPECT_EQ("Double32_t", reader->GetModel().GetConstField("d2").GetTypeAlias()); auto d1 = reader->GetModel().GetDefaultEntry().GetPtr("d1"); auto d2 = reader->GetModel().GetDefaultEntry().GetPtr("d2"); reader->LoadEntry(0); @@ -1877,8 +1878,9 @@ TEST(RNTuple, Double32Extended) auto reader = RNTupleReader::Open("ntuple", fileGuard.GetPath()); auto obj = reader->GetModel().GetDefaultEntry().GetPtr("obj"); - EXPECT_EQ("Double32_t", reader->GetModel().GetField("obj").GetSubFields()[1]->GetTypeAlias()); - EXPECT_EQ("Double32_t", reader->GetModel().GetField("obj").GetSubFields()[2]->GetSubFields()[0]->GetTypeAlias()); + EXPECT_EQ("Double32_t", reader->GetModel().GetConstField("obj").GetSubFields()[1]->GetTypeAlias()); + EXPECT_EQ("Double32_t", + reader->GetModel().GetConstField("obj").GetSubFields()[2]->GetSubFields()[0]->GetTypeAlias()); EXPECT_DOUBLE_EQ(0.0, obj->a); EXPECT_DOUBLE_EQ(1.0, obj->b); EXPECT_DOUBLE_EQ(2.0, obj->c[0]); @@ -2048,7 +2050,7 @@ TEST(RNTuple, TClassTemplateBased) auto reader = RNTupleReader::Open("f", fileGuard.GetPath()); - const auto &fieldObject = reader->GetModel().GetField("klass"); + const auto &fieldObject = reader->GetModel().GetConstField("klass"); EXPECT_EQ("EdmWrapper", fieldObject.GetTypeName()); auto object = reader->GetModel().GetDefaultEntry().GetPtr>("klass"); reader->LoadEntry(0); diff --git a/tree/ntuple/v7/test/rfield_class.cxx b/tree/ntuple/v7/test/rfield_class.cxx index e8a504a562933..939b4f1a90d66 100644 --- a/tree/ntuple/v7/test/rfield_class.cxx +++ b/tree/ntuple/v7/test/rfield_class.cxx @@ -251,7 +251,7 @@ TEST(RNTuple, TClassReadRules) auto reader = RNTupleReader::Open("f", fileGuard.GetPath()); EXPECT_EQ(5U, reader->GetNEntries()); EXPECT_EQ(TClass::GetClass("StructWithIORules")->GetCheckSum(), - reader->GetModel().GetField("class").GetOnDiskTypeChecksum()); + reader->GetModel().GetConstField("class").GetOnDiskTypeChecksum()); auto viewKlass = reader->GetView("class"); for (auto i : reader->GetEntryRange()) { float fi = static_cast(i); diff --git a/tree/ntupleutil/v7/test/ntuple_importer.cxx b/tree/ntupleutil/v7/test/ntuple_importer.cxx index 734bb8fb6abec..e9dc65b6a3c62 100644 --- a/tree/ntupleutil/v7/test/ntuple_importer.cxx +++ b/tree/ntupleutil/v7/test/ntuple_importer.cxx @@ -263,9 +263,9 @@ TEST(RNTupleImporter, FieldModifier) EXPECT_FLOAT_EQ(2.0, *reader->GetModel().GetDefaultEntry().GetPtr("b")); EXPECT_EQ(RFieldBase::ColumnRepresentation_t{EColumnType::kReal16}, - reader->GetModel().GetField("a").GetColumnRepresentatives()[0]); + reader->GetModel().GetConstField("a").GetColumnRepresentatives()[0]); EXPECT_EQ(RFieldBase::ColumnRepresentation_t{EColumnType::kSplitReal32}, - reader->GetModel().GetField("b").GetColumnRepresentatives()[0]); + reader->GetModel().GetConstField("b").GetColumnRepresentatives()[0]); } TEST(RNTupleImporter, CString) diff --git a/tree/ntupleutil/v7/test/ntuple_inspector.cxx b/tree/ntupleutil/v7/test/ntuple_inspector.cxx index 347e0fc9153a2..1d68087126856 100644 --- a/tree/ntupleutil/v7/test/ntuple_inspector.cxx +++ b/tree/ntupleutil/v7/test/ntuple_inspector.cxx @@ -774,7 +774,7 @@ TEST(RNTupleInspector, MultiColumnRepresentations) writer->Fill(); writer->CommitCluster(); ROOT::Experimental::Internal::RFieldRepresentationModifier::SetPrimaryColumnRepresentation( - const_cast(writer->GetModel().GetField("px")), 1); + const_cast(writer->GetModel().GetConstField("px")), 1); writer->Fill(); } From df519dd4cff82e4a865914c68ca750f7fd54e981 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Fri, 18 Oct 2024 12:00:16 +0200 Subject: [PATCH 2/2] [ntuple] Add RNTupleModel::GetMutableField Similar to GetMutableFieldZero(), it must only be called on unfrozen models. --- tree/ntuple/v7/inc/ROOT/RNTupleModel.hxx | 1 + tree/ntuple/v7/src/RNTupleModel.cxx | 11 +++++++++++ tree/ntuple/v7/test/ntuple_model.cxx | 8 ++++++++ 3 files changed, 20 insertions(+) diff --git a/tree/ntuple/v7/inc/ROOT/RNTupleModel.hxx b/tree/ntuple/v7/inc/ROOT/RNTupleModel.hxx index 105d3b12379fb..5a624f8584bb1 100644 --- a/tree/ntuple/v7/inc/ROOT/RNTupleModel.hxx +++ b/tree/ntuple/v7/inc/ROOT/RNTupleModel.hxx @@ -363,6 +363,7 @@ public: /// Mutable access to the root field is used to make adjustments to the fields. RFieldZero &GetMutableFieldZero(); const RFieldZero &GetConstFieldZero() const { return *fFieldZero; } + RFieldBase &GetMutableField(std::string_view fieldName); const RFieldBase &GetConstField(std::string_view fieldName) const; const std::string &GetDescription() const { return fDescription; } diff --git a/tree/ntuple/v7/src/RNTupleModel.cxx b/tree/ntuple/v7/src/RNTupleModel.cxx index 902d5ca1d359b..e536686ce5422 100644 --- a/tree/ntuple/v7/src/RNTupleModel.cxx +++ b/tree/ntuple/v7/src/RNTupleModel.cxx @@ -354,6 +354,17 @@ ROOT::Experimental::RFieldZero &ROOT::Experimental::RNTupleModel::GetMutableFiel return *fFieldZero; } +ROOT::Experimental::RFieldBase &ROOT::Experimental::RNTupleModel::GetMutableField(std::string_view fieldName) +{ + if (IsFrozen()) + throw RException(R__FAIL("invalid attempt to get mutable field of frozen model")); + auto f = FindField(fieldName); + if (!f) + throw RException(R__FAIL("invalid field: " + std::string(fieldName))); + + return *f; +} + const ROOT::Experimental::RFieldBase &ROOT::Experimental::RNTupleModel::GetConstField(std::string_view fieldName) const { auto f = FindField(fieldName); diff --git a/tree/ntuple/v7/test/ntuple_model.cxx b/tree/ntuple/v7/test/ntuple_model.cxx index 507f7697363d9..d3dd7b51c9be8 100644 --- a/tree/ntuple/v7/test/ntuple_model.cxx +++ b/tree/ntuple/v7/test/ntuple_model.cxx @@ -67,11 +67,13 @@ TEST(RNTupleModel, GetField) auto m = RNTupleModel::Create(); m->MakeField("x"); m->MakeField("cs"); + m->GetMutableField("cs.v1._0").SetColumnRepresentatives({{EColumnType::kReal32}}); m->Freeze(); EXPECT_EQ(m->GetConstField("x").GetFieldName(), "x"); EXPECT_EQ(m->GetConstField("x").GetTypeName(), "std::int32_t"); EXPECT_EQ(m->GetConstField("cs.v1").GetFieldName(), "v1"); EXPECT_EQ(m->GetConstField("cs.v1").GetTypeName(), "std::vector"); + EXPECT_EQ(m->GetConstField("cs.v1._0").GetColumnRepresentatives()[0][0], EColumnType::kReal32); try { m->GetConstField("nonexistent"); FAIL() << "invalid field name should throw"; @@ -90,6 +92,12 @@ TEST(RNTupleModel, GetField) } catch (const RException &err) { EXPECT_THAT(err.what(), testing::HasSubstr("frozen model")); } + try { + m->GetMutableField("x"); + FAIL() << "GetMutableField should throw"; + } catch (const RException &err) { + EXPECT_THAT(err.what(), testing::HasSubstr("frozen model")); + } EXPECT_EQ("", m->GetConstFieldZero().GetQualifiedFieldName()); EXPECT_EQ("x", m->GetConstField("x").GetQualifiedFieldName()); EXPECT_EQ("cs", m->GetConstField("cs").GetQualifiedFieldName());