Bugs
The code has at least two bugs.
The first is when two adjacent list items are more than 6 months old, the second one will not be deleted. This occurs because when an item is deleted, the rest of the list items gets moved forward. When the item at index i
is deleted, the items from index i+1
to the end get copied up one position. That is, the item at index i+1
gets copied to index i
, etc. The for
-loop then increments i
and the item that was previously at index i+1
gets skipped.
The second bug is obvious. Any time a item is deleted from the list, an IndexError will be raised. When an item is deleted, the list is shortened. But the for
-loop runs over the length of the original list, so i
will run past the end of the shortened list.
Both bugs can be seen in this simple example:
test = [1,5,6,3,2]
for i in range(len(test)):
print(i, test)
if test[i]>3:
del test[i]
Output:
0 [1, 5, 6, 3, 2]
1 [1, 5, 6, 3, 2]
2 [1, 6, 3, 2]
3 [1, 6, 3, 2]
4 [1, 6, 3, 2]
---------------------------------------------------------------------------
IndexError Traceback (most recent call last)
The code deletes items > 3, but misses the '6' following the '5'. It also raised an IndexError
.
One way to fix the code is to iterate over it in reverse:
test = [1,5,6,3,2]
for i in range(len(test)-1, -1, -1):
print(i, test)
if test[i]>3:
del test[i]
print(test)
Outputs:
4 [1, 5, 6, 3, 2]
3 [1, 5, 6, 3, 2]
2 [1, 5, 6, 3, 2]
1 [1, 5, 3, 2]
0 [1, 3, 2]
[1, 3, 2]
This works because the portion of the list that gets moved because of a deletion have already been checked. And even if the list is shortened, it first index is always 0
.
Performance
The current algorithm is O(n^2). The list comprehension proposed by am_an_attendant_lord is O(n), but uses additional memory. An O(n) in-place algorithm could look something like:
test = [1,5,6,3,2]
i = 0
for item in test:
print(i, test)
if item <= 3:
test[i] = item
i += 1
del test[i:]
print(test)
Compute things that don't change before the loop instead of in the loop. For example,
gp_repeat_medications_list = gp_clinicals_meds_repeat["GpRepeatMedicationsList"]`
and
cutoff_date = (datetime.now() - timedelta(days=180)).date()
isoformat dates
The parts of an ISO formatted date string are in order of significance. The year comes first and the seconds come last. This is useful because the date strings can be compared directly with out converting the strings to datetime
objects. In my testing, converting to datetime
objects and comparing them takes 4 times longer than comparing the strings. (This presumes all the strings have the same time zone or all have no time zone).
Conclusion
Combine the above and get something like this
gp_clinicals_meds_repeat = session["gp_clinicals_meds_repeat"]
gp_repeat_medications_list = gp_clinicals_meds_repeat["GpRepeatMedicationsList"]
cutoff_date = (datetime.now() - timedelta(days=180)).date().isoformat()
i = 0
for item in gp_repeat_medications_list:
if item["Date"] >= cutoff_date:
gp_repeat_medications_list[i] = item
i += 1
del gp_repeat_medications_list[i:]
session
? Can these data be loaded into Pandas? \$\endgroup\$