fix issue 49 get/mget with empty series #50
Conversation
redistimeseries/client.py
Outdated
| def parse_m_get(response): | ||
| res = [] | ||
| for item in response: | ||
| res.append({ nativestr(item[0]) : [list_to_dict(item[1]), | ||
| item[2][0], float(item[2][1])]}) | ||
| if len(item[2]) is 2: |
There was a problem hiding this comment.
Why are numerical values compared via the "is" operator and not through "=="?
In this case, it will give the correct result, but it seems to me that using "is" is conceptually wrong here.
If I'm not mistaken, then python keeps an array of integer objects for all integers between -5 and 256.
This may lead to an unexpected result if the numerical variable is not in the range of -5..256.
l1 = list(range(2))
l2 = list(range(257))
print(len(l1) == 2) #True
print(len(l1) is 2) #True
print(len(l2) == 257) #True
print(len(l2) is 257) #False
I do not have much experience with this, but it seems to me to use "==" to compare numerical values.
Codecov Report
@@ Coverage Diff @@
## master #50 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 310 322 +12
=========================================
+ Hits 310 322 +12 Continue to review full report at Codecov.
|
|
This pull request introduces 1 alert when merging 2e23dc8 into d9a2246 - view on LGTM.com new alerts:
|
redistimeseries/client.py
Outdated
| @@ -42,11 +42,19 @@ def parse_m_range(response): | |||
| parse_range(item[2])]}) | |||
| return res | |||
|
|
|||
| def parse_get(response): | |||
| if response[0] == None: | |||
There was a problem hiding this comment.
@ashtul the LGTM bot complains about this because python linting rules prefer to use if response[0] is None.
fix #49
Made the fix but it should probably be fixed at the module level as well.
Created PR RedisTimeSeries/RedisTimeSeries#346 which requires another change here