From 8d7c3a416134ce3e00fa6433918e868a8c57bff4 Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller Date: Thu, 22 Oct 2026 14:37:46 +0200 Subject: [PATCH 1/4] [PyROOT] Take ownership of objects returned by TDirectoryFile.Get that are not owned by the directory --- .../ROOT/_pythonization/_tdirectoryfile.py | 7 ++- .../pythonizations/test/tdirectory_tfile.py | 43 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_tdirectoryfile.py b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_tdirectoryfile.py index 0c9d0e41e4874..2066d8fde89bd 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_tdirectoryfile.py +++ b/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_tdirectoryfile.py @@ -69,7 +69,12 @@ def _TDirectoryFile_Get(self, namecycle): if key: class_name = key.GetClassName() address = self.GetObjectChecked(namecycle, class_name) - return ROOT._cppyy.bind_object(address, class_name) + obj = ROOT._cppyy.bind_object(address, class_name) + # Objects that are not registered in the directory (e.g. TGraph or + # non-TObjects like ROOT::RNTuple) are owned by the caller + if address and not (isinstance(obj, ROOT.TObject) and self.GetList().FindObject(obj)): + ROOT.SetOwnership(obj, True) + return obj # no key? for better or worse, call normal Get() return self._Get(namecycle) diff --git a/bindings/pyroot/pythonizations/test/tdirectory_tfile.py b/bindings/pyroot/pythonizations/test/tdirectory_tfile.py index 64ec169f7881d..6f1e1b8a07928 100644 --- a/bindings/pyroot/pythonizations/test/tdirectory_tfile.py +++ b/bindings/pyroot/pythonizations/test/tdirectory_tfile.py @@ -210,6 +210,49 @@ def test_keys_title(self): finput.Close() +class TFileGetOwnership(unittest.TestCase): + """ + Test that the objects returned by Get are owned by Python unless they are + owned by the directory (#16178) + """ + + filename = "tfilegetownership.root" + + @classmethod + def setUpClass(cls): + with TFile(cls.filename, "recreate") as f: + ROOT.TH1F("h", "h", 10, 0, 1).Write() + ROOT.TTree("t", "t").Write() + f.mkdir("dir") + ROOT.TGraph(10).Write("g") + ROOT.TObjString("s").Write("s") + f.WriteObject(ROOT.std.vector["double"](10), "v") + + @classmethod + def tearDownClass(cls): + os.remove(cls.filename) + + def test_owned_by_directory(self): + with TFile(self.filename) as f: + for name in ["h", "t", "dir"]: + obj = f.Get(name) + self.assertFalse(obj.__python_owns__, name) + del obj + # The object is still alive in the directory + self.assertTrue(f.GetList().FindObject(name), name) + self.assertEqual(f.Get(name).GetName(), name) + + def test_owned_by_python(self): + with TFile(self.filename) as f: + for name in ["g", "s", "v"]: + obj = f.Get(name) + self.assertTrue(obj.__python_owns__, name) + self.assertFalse(f.GetList().FindObject(name), name) + self.assertEqual(f.Get("g").GetN(), 10) + self.assertEqual(f.Get("s").GetString(), "s") + self.assertEqual(f.Get("v").size(), 10) + + class TFileConstructor(unittest.TestCase): """ Test for the TFile constructor From e6485c24a5fd9f7ef04898b98d1173f799573817 Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller <22276694+jmcarcell@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:31:41 +0200 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Vincenzo Eduardo Padulano --- bindings/pyroot/pythonizations/test/tdirectory_tfile.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bindings/pyroot/pythonizations/test/tdirectory_tfile.py b/bindings/pyroot/pythonizations/test/tdirectory_tfile.py index 6f1e1b8a07928..17d5ad908801c 100644 --- a/bindings/pyroot/pythonizations/test/tdirectory_tfile.py +++ b/bindings/pyroot/pythonizations/test/tdirectory_tfile.py @@ -235,11 +235,9 @@ def tearDownClass(cls): def test_owned_by_directory(self): with TFile(self.filename) as f: for name in ["h", "t", "dir"]: - obj = f.Get(name) - self.assertFalse(obj.__python_owns__, name) - del obj + self.assertFalse(f.Get(name).__python_owns__, name) # The object is still alive in the directory - self.assertTrue(f.GetList().FindObject(name), name) + self.assertTrue(f.GetList().FindObject(name), msg=name) self.assertEqual(f.Get(name).GetName(), name) def test_owned_by_python(self): From 634c7eedf79a352b830315da4858531ea097a069 Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller Date: Thu, 22 Oct 2026 16:02:31 +0200 Subject: [PATCH 3/4] [PyROOT] Use the objects returned by Get after closing the file in the ownership test --- .../pythonizations/test/tdirectory_tfile.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/bindings/pyroot/pythonizations/test/tdirectory_tfile.py b/bindings/pyroot/pythonizations/test/tdirectory_tfile.py index 17d5ad908801c..89e7160c885fe 100644 --- a/bindings/pyroot/pythonizations/test/tdirectory_tfile.py +++ b/bindings/pyroot/pythonizations/test/tdirectory_tfile.py @@ -235,20 +235,21 @@ def tearDownClass(cls): def test_owned_by_directory(self): with TFile(self.filename) as f: for name in ["h", "t", "dir"]: - self.assertFalse(f.Get(name).__python_owns__, name) + self.assertFalse(f.Get(name).__python_owns__, msg=name) # The object is still alive in the directory self.assertTrue(f.GetList().FindObject(name), msg=name) self.assertEqual(f.Get(name).GetName(), name) def test_owned_by_python(self): with TFile(self.filename) as f: - for name in ["g", "s", "v"]: - obj = f.Get(name) - self.assertTrue(obj.__python_owns__, name) - self.assertFalse(f.GetList().FindObject(name), name) - self.assertEqual(f.Get("g").GetN(), 10) - self.assertEqual(f.Get("s").GetString(), "s") - self.assertEqual(f.Get("v").size(), 10) + objs = {name: f.Get(name) for name in ["g", "s", "v"]} + for name, obj in objs.items(): + self.assertTrue(obj.__python_owns__, msg=name) + self.assertFalse(f.GetList().FindObject(name), msg=name) + # The objects are still usable after the file is closed + self.assertEqual(objs["g"].GetN(), 10) + self.assertEqual(objs["s"].GetString(), "s") + self.assertEqual(objs["v"].size(), 10) class TFileConstructor(unittest.TestCase): From 2b10086c6407cede6ec9bfac9e19845253021d14 Mon Sep 17 00:00:00 2001 From: Juan Miguel Carceller Date: Fri, 23 Oct 2026 11:52:25 +0200 Subject: [PATCH 4/4] [PyROOT] Check the ownership against the registration in the directory in the test Assisted-by: claude-code:claude-opus-5-5 --- .../pyroot/pythonizations/test/tdirectory_tfile.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/bindings/pyroot/pythonizations/test/tdirectory_tfile.py b/bindings/pyroot/pythonizations/test/tdirectory_tfile.py index 89e7160c885fe..bbcb5af5b1710 100644 --- a/bindings/pyroot/pythonizations/test/tdirectory_tfile.py +++ b/bindings/pyroot/pythonizations/test/tdirectory_tfile.py @@ -235,9 +235,14 @@ def tearDownClass(cls): def test_owned_by_directory(self): with TFile(self.filename) as f: for name in ["h", "t", "dir"]: - self.assertFalse(f.Get(name).__python_owns__, msg=name) - # The object is still alive in the directory - self.assertTrue(f.GetList().FindObject(name), msg=name) + obj = f.Get(name) + # Whether these objects are registered in the directory may + # depend on ROOT::Experimental::ObjectAutoRegistrationEnabled() + # in the future, so check the ownership against the actual + # registration + registered = bool(f.GetList().FindObject(name)) + self.assertEqual(obj.__python_owns__, not registered, msg=name) + # The object is still alive self.assertEqual(f.Get(name).GetName(), name) def test_owned_by_python(self):