Update limit.py #374
Update limit.py #374
Conversation
there are some syntax warnings in this code. there maybe another logical error: in this code, when parameter min_lim and parameter max_lim are both None, it will not hit any if judgment, result will return []. however it should return arr[::] actually.
Pull Request Test Coverage Report for Build 673
💛 - Coveralls |
| result.append(i) | ||
| elif max_lim == None: | ||
| elif min_lim and max_lim is None: |
There was a problem hiding this comment.
@ggran-REN The syntax warnings maybe in these if conditions. What if min_lim is 0, then the code fails.
Consider the case where I give limit(arr, 0, 0), I would expect [], but will get same array instead.
All this can simply be solved by:
def limit(arr, min_lim=None, max_lim=None):
if min_lim is None:
min_lim = float("-infinity")
if max_lim is None:
max_lim = float("infinity")
result = []
for i in arr:
if min_lim <= i <= max_lim:
result.append(i)
return resultThere was a problem hiding this comment.
Thasks alot ：-）！
i overlooked 0 is always False in python~
There was a problem hiding this comment.
@ggran-REN I see that you are a first-time contributor, its great :).
BTW, you can fix the code in your branch ggran-REN:patch-1 to reflect changes in this PR. Thanks.
| if i < min_lim: | ||
| continue | ||
| if i > max_lim: | ||
| continue |
There was a problem hiding this comment.
nit
you could probably merge these two ifs into a single if statement.
|
There is currently a shorter implementation of limit.py in arrays so we should close this PR. |
there are some syntax warnings in this code.
there maybe another logical error:
in this code, when parameter min_lim and parameter max_lim are both None, it will not hit any if judgment, result will return [].
however it should return arr[::] actually.
(Put an
Xinside the[ ]to denote check mark[X].)If creating a new file :
if done some changes :
other