openPMD / openPMD-api

:floppy_disk: C++ & Python API for Scientific I/O
https://openpmd-api.readthedocs.io
GNU Lesser General Public License v3.0
142 stars 51 forks source link

Python Series/Iteration: Fix Length #1659

Closed ax3l closed 2 months ago

ax3l commented 3 months ago

If the len(...) intrinsic for series and iterations. Close #1375

import openpmd_api as io

s = io.Series("build/samples/git-sample/3d-bp4/example-3d-bp4_%T.bp", io.Access.read_only)

s
# <openPMD.Series at 'build/samples/git-sample/3d-bp4/example-3d-bp4_%T.bp' with 2 iteration(s) and 10 attributes>

s.iterations
# <openPMD.Iteration_Container with 2 entries and 0 attribute(s)>

len(s)
# 2

len(s.iterations)
# 2
ax3l commented 3 months ago

I am surprised the Python tests pass. I would have at least expected these to fail:

diff --git a/test/python/unittest/API/APITest.py b/test/python/unittest/API/APITest.py
index 59e6b5c9..f5bc5557 100644
--- a/test/python/unittest/API/APITest.py
+++ b/test/python/unittest/API/APITest.py
@@ -87,11 +87,12 @@ class APITest(unittest.TestCase):
         series = self.__field_series

         self.assertEqual(series.openPMD, "1.0.0")
-        self.assertEqual(len(series.iterations), 1)
+        self.assertEqual(len(series), 6)  # was 1
+        self.assertEqual(len(series.iterations), 6)
         for i in series.iterations:
             self.assertTrue(i in [0, 100, 200, 300, 400, 500])

-        self.assertEqual(len(series.iterations), 1)
+        self.assertEqual(len(series.iterations), 6)  # was 1

         self.assertTrue(400 in series.iterations)
         i = series.iterations[400]
franzpoeschel commented 2 months ago

Should we even keep .def("__len__", &Attributable::numAttributes) in src/binding/python/Attributable.cpp at this point? It's only not overwritten for RecordComponents, and it's rather surprising that len would suddenly return the number of attributes there.

franzpoeschel commented 2 months ago

I am surprised the Python tests pass. I would have at least expected these to fail:

diff --git a/test/python/unittest/API/APITest.py b/test/python/unittest/API/APITest.py
index 59e6b5c9..f5bc5557 100644
--- a/test/python/unittest/API/APITest.py
+++ b/test/python/unittest/API/APITest.py
@@ -87,11 +87,12 @@ class APITest(unittest.TestCase):
         series = self.__field_series

         self.assertEqual(series.openPMD, "1.0.0")
-        self.assertEqual(len(series.iterations), 1)
+        self.assertEqual(len(series), 6)  # was 1
+        self.assertEqual(len(series.iterations), 6)
         for i in series.iterations:
             self.assertTrue(i in [0, 100, 200, 300, 400, 500])

-        self.assertEqual(len(series.iterations), 1)
+        self.assertEqual(len(series.iterations), 6)  # was 1

         self.assertTrue(400 in series.iterations)
         i = series.iterations[400]

When I apply this diff locally, the modified tests do fail