)]}'
{"oslo_versionedobjects/base.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a29ed9ba857b2c1e8a55dbcd87cf0a40ccb5b6a3","unresolved":false,"context_lines":[{"line_number":676,"context_line":"            return list(self.itervalues())"},{"line_number":677,"context_line":""},{"line_number":678,"context_line":"        def items(self):"},{"line_number":679,"context_line":"            return list(self.iteritems())"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":"    def __len__(self):"},{"line_number":682,"context_line":"        fields \u003d 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a0c5dc1_9dab861d","line":679,"updated":"2015-05-13 15:20:07.000000000","message":"I really hate this if statement in the middle of the class like this.\n\nHow about we create two classes (that inherit from a common base if you like) that give the proper behavior(s) and then set the name to one of those based on the version?\n\n if six.PY3:\n     VersionedObjectDictCompat \u003d Py3VersionedObjectDictCompat\n else:\n     VersionedObjectDictCompat \u003d Py2VersionedObjectDictCompat\n\n?","commit_id":"e5557dc4883f39d60dd66adcc74d29f40d53a156"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"10f9a6569dda1c657f23c48e08ac41e6adf7fa06","unresolved":false,"context_lines":[{"line_number":676,"context_line":"            return list(self.itervalues())"},{"line_number":677,"context_line":""},{"line_number":678,"context_line":"        def items(self):"},{"line_number":679,"context_line":"            return list(self.iteritems())"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":"    def __len__(self):"},{"line_number":682,"context_line":"        fields \u003d 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a0c5dc1_6ad92583","line":679,"in_reply_to":"9a0c5dc1_9dab861d","updated":"2015-05-13 16:13:43.000000000","message":"Except of these 3 methods, all the code is shared between Python 2 and Python 3. It means that a base class is needed, and so that 3 classes are needed + an alias: 4 names for the \"same thing\" :-/ I prefer putting an if in the class body or inside methods.\n\nPutting the if inside the class body has no performance overhead at runtime (only when importing the module, so only once). If I move the if inside methods, it would have a small overhead at each call.\n\nPutting if in the body class is not so uncommon. Some examples in the Python stdlib:\n\n* https://hg.python.org/cpython/file/c8adc2c13c8b/Lib/asyncio/tasks.py#l86\n* https://hg.python.org/cpython/file/c8adc2c13c8b/Lib/lib2to3/pytree.py#l204\n\nPython stdlib is not the best example, since it\u0027s rare that code is written for multiple Python versions. It\u0027s more common in third party modules which have to support a wide range of Python versions.","commit_id":"e5557dc4883f39d60dd66adcc74d29f40d53a156"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a29ed9ba857b2c1e8a55dbcd87cf0a40ccb5b6a3","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        fields \u003d 0"},{"line_number":683,"context_line":"        for name in self:"},{"line_number":684,"context_line":"            fields +\u003d 1"},{"line_number":685,"context_line":"        return fields"},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"    def __getitem__(self, name):"},{"line_number":688,"context_line":"        return getattr(self, name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a0c5dc1_fdd4c2bd","line":685,"updated":"2015-05-13 15:20:07.000000000","message":"Same comment as before: why not just:\n\n return len(self.fields)\n\n?","commit_id":"e5557dc4883f39d60dd66adcc74d29f40d53a156"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a7c5cf58aa515503fea5aa165174838a0b733f84","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        fields \u003d 0"},{"line_number":683,"context_line":"        for name in self:"},{"line_number":684,"context_line":"            fields +\u003d 1"},{"line_number":685,"context_line":"        return fields"},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"    def __getitem__(self, name):"},{"line_number":688,"context_line":"        return getattr(self, name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a0c5dc1_d51b127f","line":685,"in_reply_to":"9a0c5dc1_47e89c10","updated":"2015-05-13 16:53:17.000000000","message":"Dropping it, yeah.","commit_id":"e5557dc4883f39d60dd66adcc74d29f40d53a156"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"10f9a6569dda1c657f23c48e08ac41e6adf7fa06","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        fields \u003d 0"},{"line_number":683,"context_line":"        for name in self:"},{"line_number":684,"context_line":"            fields +\u003d 1"},{"line_number":685,"context_line":"        return fields"},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"    def __getitem__(self, name):"},{"line_number":688,"context_line":"        return getattr(self, name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a0c5dc1_47e89c10","line":685,"in_reply_to":"9a0c5dc1_fdd4c2bd","updated":"2015-05-13 16:13:43.000000000","message":"I expected len(obj) to be the same than list(obj). __iter__ skips fields which are not set (except of extra fields).\n\nSee the unit test which expects a length of 2 items, whereas there are 7 fields.\n\nWell, it\u0027s not completly clear for me if VersionedObjectDictCompat is supposed to implement the exact dictionary API. For example, get() raises AttributeError for missing key, whereas dict.get() raises KeyError.\n\nI just checked: the __len__() method is not needed to support dict(obj) syntax, so maybe we can just drop it. What do you think?","commit_id":"e5557dc4883f39d60dd66adcc74d29f40d53a156"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a29ed9ba857b2c1e8a55dbcd87cf0a40ccb5b6a3","unresolved":false,"context_lines":[{"line_number":688,"context_line":"        return getattr(self, name)"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"    def __setitem__(self, name, value):"},{"line_number":691,"context_line":"        setattr(self, name, value)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def __contains__(self, name):"},{"line_number":694,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a0c5dc1_3d35baf6","line":691,"updated":"2015-05-13 15:20:07.000000000","message":"I guess you don\u0027t have to do it in this patch, but I liked getting __delitem__ support from the previous iteration. Now that I wrote support for you, any reason not to include it here?","commit_id":"e5557dc4883f39d60dd66adcc74d29f40d53a156"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"10f9a6569dda1c657f23c48e08ac41e6adf7fa06","unresolved":false,"context_lines":[{"line_number":688,"context_line":"        return getattr(self, name)"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"    def __setitem__(self, name, value):"},{"line_number":691,"context_line":"        setattr(self, name, value)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def __contains__(self, name):"},{"line_number":694,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a0c5dc1_67e3a006","line":691,"in_reply_to":"9a0c5dc1_3d35baf6","updated":"2015-05-13 16:13:43.000000000","message":"The full dictionary API requires to implement much more  methods. Examples: pop(), popitem(), remove(), clear(), setdefault(), etc.\n\nI don\u0027t think that we need the full API, I guess that obj[key] and dict(obj) are enough for most use cases.\n\nI didn\u0027t want to add __delitem__ method. My previous try using MutableMapping ABC forced me to implement the method.\n\nI chose to create a new fresh change instead of reusing the previous one because my new approach is vey different.","commit_id":"e5557dc4883f39d60dd66adcc74d29f40d53a156"}]}