Repository: zhmcclient/python-zhmcclient

Issue:
TimeStatsKeeper.snapshot() result not very handy
The `TimeStatsKeeper.snapshot()` method returns a list of tuples (op_name, TimeStats), which it produces by appliying the `items()` method on the internal dictionary.

Most uses of the snapshot I have seen so far, transform that back into a dictionary by using `dict(snapshot)`.

It would be more efficient and more convenient if snapshot() simply returned (a copy of) the dictionary directly.

Note that this would be an incompatible change.

Discussion:
GitCop: There were the following issues with your Pull Request

 * Commit: a2e553b013c36998d69c086281ac2156d71481d3
  * Your commit message body contains a line that is longer than 80 characters

Guidelines are available at https://github.com/zhmcclient/python-zhmcclient/blob/master/CONTRIBUTING.rst

---
This message was auto-generated by https://gitcop.com

GitCop: There were the following issues with your Pull Request

 * Commit: 56a1eaed0dc89da51487a141e15dc7912f86a9af
  * Your commit message body contains a line that is longer than 80 characters

Guidelines are available at https://github.com/zhmcclient/python-zhmcclient/blob/master/CONTRIBUTING.rst

---
This message was auto-generated by https://gitcop.com


Patch:
diff --git a/docs/changes.rst b/docs/changes.rst
index f6dccff..51adb06 100755
--- a/docs/changes.rst
+++ b/docs/changes.rst
@@ -27,6 +27,10 @@ Released: not yet
 
 **Incompatible changes:**
 
+* Changed the return value of ``TimeStatsKeeper.snapshot()`` from a list of
+  key/value tuples to a dictionary. This is more flexible and reduces the
+  number of data structure conversions in different scenarios. See issue #269.
+
 **Deprecations:**
 
 **Bug fixes:**
diff --git a/zhmcclient/_timestats.py b/zhmcclient/_timestats.py
index e0d9c3d..07dc648 100644
--- a/zhmcclient/_timestats.py
+++ b/zhmcclient/_timestats.py
@@ -280,13 +280,13 @@ class TimeStatsKeeper(object):
 
         Returns:
 
-          : A list of tuples (name, stats) with:
+         dict: A dictionary of the time statistics by operation, where:
 
-          - name (:term:`string`): Name of the operation
-          - stats (:class:`~zhmcclient.TimeStats`): Time statistics for the
+          - key (:term:`string`): Name of the operation
+          - value (:class:`~zhmcclient.TimeStats`): Time statistics for the
             operation
         """
-        return copy.deepcopy(self._time_stats).items()
+        return copy.deepcopy(self._time_stats)
 
     def __str__(self):
         """
@@ -303,7 +303,8 @@ class TimeStatsKeeper(object):
         ret = "Time statistics (times in seconds):\n"
         if self.enabled:
             ret += "Count  Average  Minimum  Maximum  Operation name\n"
-            snapshot_by_avg = sorted(self.snapshot(),
+            stats_dict = self.snapshot()
+            snapshot_by_avg = sorted(stats_dict.items(),
                                      key=lambda item: item[1].avg_time,
                                      reverse=True)
             for name, stats in snapshot_by_avg:


Test patch:
diff --git a/tests/unit/test_timestats.py b/tests/unit/test_timestats.py
index 1155489..1733f6f 100755
--- a/tests/unit/test_timestats.py
+++ b/tests/unit/test_timestats.py
@@ -142,7 +142,9 @@ class TimeStatsTests(unittest.TestCase):
         stats = keeper.get_stats('foo')
         dur = measure(stats, duration)
 
-        for _, stats in keeper.snapshot():
+        stats_dict = keeper.snapshot()
+        for op_name in stats_dict:
+            stats = stats_dict[op_name]
             self.assertEqual(stats.count, 1)
             self.assertLess(time_abs_delta(stats.avg_time, dur), delta,
                             "avg time: actual: %f, expected: %f, delta: %f" %
@@ -174,7 +176,9 @@ class TimeStatsTests(unittest.TestCase):
         time.sleep(duration)
         stats.end()
 
-        for _, stats in keeper.snapshot():
+        stats_dict = keeper.snapshot()
+        for op_name in stats_dict:
+            stats = stats_dict[op_name]
             self.assertEqual(stats.count, 0)
             self.assertEqual(stats.avg_time, 0)
             self.assertEqual(stats.min_time, float('inf'))
@@ -196,7 +200,7 @@ class TimeStatsTests(unittest.TestCase):
         stats.end()
 
         # take the snapshot
-        snapshot = keeper.snapshot()
+        snap_stats_dict = keeper.snapshot()
 
         # produce a second data item
         stats.begin()
@@ -204,7 +208,8 @@ class TimeStatsTests(unittest.TestCase):
         stats.end()
 
         # verify that only the first data item is in the snapshot
-        for _, snap_stats in snapshot:
+        for op_name in snap_stats_dict:
+            snap_stats = snap_stats_dict[op_name]
             self.assertEqual(snap_stats.count, 1)
 
         # verify that both data items are in the original stats object
@@ -230,7 +235,9 @@ class TimeStatsTests(unittest.TestCase):
         max_dur = max(m_durations)
         avg_dur = sum(m_durations) / float(count)
 
-        for _, stats in keeper.snapshot():
+        stats_dict = keeper.snapshot()
+        for op_name in stats_dict:
+            stats = stats_dict[op_name]
             self.assertEqual(stats.count, 3)
             self.assertLess(time_abs_delta(stats.avg_time, avg_dur), delta,
                             "avg time: actual: %f, expected: %f, delta: %f" %