fix RuntimeError with Python 3.5 #27
Conversation
kophy
commented
May 20, 2017
kophy
commented
- fix Python 3.5 RuntimeError mentioned in issue RuntimeError with Python3.5 #25
- add callback support for expired items (personally think it's useful)
|
It is better not to mix a bug fix and a feature proposal in one PR. |
|
Sure. Shall I remove callback part and make a new PR later? @horkhe |
|
yes, please |
kophy
changed the title
| @@ -69,7 +69,10 @@ def __setitem__(self, key, value): | |||
| """ Set d[key] to value. """ | |||
| with self.lock: | |||
| if len(self) == self.max_len: | |||
| self.popitem(last=False) | |||
| try: | |||
| self.popitem(last=False) | |||
There was a problem hiding this comment.
I assume that python 3.x raises exception if it is empty? If so then it should be a specific exception type so please catch only that specific exception in the except clause below.
expiringdict/__init__.py
Outdated
| @@ -110,7 +113,7 @@ def get(self, key, default=None, with_age=False): | |||
| def items(self): | |||
| """ Return a copy of the dictionary's list of (key, value) pairs. """ | |||
| r = [] | |||
| for key in self: | |||
| for key in list(self.keys()): | |||
There was a problem hiding this comment.
I am not familiar with python 3.x, why do we need list here? Anyway even if it is needed for 3.x it is definitely not needed in 2.x and it would be better not to have this overhead when it is not needed.
There was a problem hiding this comment.
That makes sense. I will check version first.
In python 3.5, there are some implementation changes of OrderedDict, so directly iterating will give RuntimeError: OrderedDict mutated during iteration.
expiringdict/__init__.py
Outdated
| r.append((key, self[key])) | ||
| except KeyError: | ||
| pass | ||
| if sys.version_info >= (3, 5): |
There was a problem hiding this comment.
I am not very fond of this solution for two reasons. Code duplication and the version check is performed every time when we call the method. How about we create _safe_keys alias at the class level:
if sys.version_info >= (3, 5):
_safe_keys = lambda self: list(self.keys())
else:
_safe_keys = keysAnd will use it as follows:
def items(self):
""" Return a copy of the dictionary's list of (key, value) pairs. """
r = []
for key in self._safe_keys():
try:
r.append((key, self[key]))
except KeyError:
pass
return r
In this case version check is performed only once when the class is imported and code duplication is avoided.
There was a problem hiding this comment.
Yes, that's certainly a better solution, but the lambda function will give TypeError under python 3. I will try to solve this.
|
@kophy please squash the changes and I will merge the PR. |
