.#3
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements solutions for three coding exercises in analytics.py and adds a practice file. The PR attempts to implement list filtering, product code counting, and moving average calculation functions.
- Implements
filter_sales_above_threshold()with critical logic bugs - Implements
count_product_codes()with correct logic but unreachable code - Implements
calculate_moving_average()with incorrect window calculation logic
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| analytics.py | Adds implementations for three functions (filter_sales_above_threshold, count_product_codes, calculate_moving_average) with multiple bugs and style issues |
| practise.py | Adds a new practice/scratchpad file with a calculate_moving_average implementation that has similar bugs to analytics.py |
| .vscode/settings.json | Configures Python unittest settings for the project |
| pycache/test_analytics.cpython-312.pyc | Generated Python bytecode file from test execution |
| pycache/analytics.cpython-312.pyc | Generated Python bytecode file from analytics module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| total=sum(numbers) | ||
| result=total/window_size |
There was a problem hiding this comment.
Missing spacing around the assignment and division operators. Should be result = total / window_size according to PEP 8 style guidelines.
| total=sum(numbers) | |
| result=total/window_size | |
| total = sum(numbers) | |
| result = total / window_size |
| """ | ||
| # # TODO: Write your code here | ||
| # pass | ||
| total=sum(numbers) |
There was a problem hiding this comment.
Missing spacing around the assignment operator. Should be total = sum(numbers) according to PEP 8 style guidelines.
| total=sum(numbers) | |
| total = sum(numbers) |
| list=[] | ||
| for i in list: | ||
| if i>threshold: | ||
| return i | ||
| else: | ||
| return list |
There was a problem hiding this comment.
Using list as a variable name shadows Python's built-in list type, which is a common anti-pattern and violates Python naming best practices. Use a descriptive name like result or filtered_sales instead.
| list=[] | |
| for i in list: | |
| if i>threshold: | |
| return i | |
| else: | |
| return list | |
| filtered_sales = [] | |
| for sale in sales: | |
| if sale > threshold: | |
| filtered_sales.append(sale) | |
| return filtered_sales |
| # # TODO: Write your code here | ||
| # pass | ||
| total=sum(numbers) | ||
| result=total/window_size | ||
| return result |
There was a problem hiding this comment.
The implementation has multiple bugs that fail the documented requirements:
- Incorrect window calculation: The function sums all elements in
numbersinstead of only the lastwindow_sizeelements. It should usenumbers[-window_size:]. - Missing edge case handling: Returns
ZeroDivisionErrorfor empty lists instead of0.0as specified. - Missing rounding: The result should be rounded to 2 decimal places using
round(result, 2). - Wrong logic for fewer elements: When the list has fewer elements than
window_size, it incorrectly divides bywindow_sizeinstead of the actual number of elements.
| # # TODO: Write your code here | |
| # pass | |
| total=sum(numbers) | |
| result=total/window_size | |
| return result | |
| if not numbers: | |
| return 0.0 | |
| window = numbers[-window_size:] | |
| avg = sum(window) / len(window) | |
| return round(avg, 2) |
| for i in list: | ||
| if i>threshold: | ||
| return i |
There was a problem hiding this comment.
Variable name i is not descriptive for this context. Use a more meaningful name like sale or amount to improve code readability.
| for i in list: | |
| if i>threshold: | |
| return i | |
| for sale in list: | |
| if sale>threshold: | |
| return sale |
|
|
||
| # # TODO: Write your code here | ||
| # pass | ||
| total=sum(numbers) |
There was a problem hiding this comment.
Missing spacing around the assignment operator. Should be total = sum(numbers) according to PEP 8 style guidelines.
| total=sum(numbers) | |
| total = sum(numbers) |
| @@ -34,6 +42,13 @@ def count_product_codes(codes: list, prefix: str): | |||
| """ | |||
| # TODO: Write your code here | |||
| pass | |||
There was a problem hiding this comment.
Unreachable code: The pass statement on line 44 is never executed because the code after it (lines 45-49) will always run first. Since lines 45-49 contain the actual implementation, this pass statement should be removed.
| pass |
| # # TODO: Write your code here | ||
| # pass | ||
| total=sum(numbers) | ||
| result=total/window_size |
There was a problem hiding this comment.
Missing spacing around the assignment and division operators. Should be result = total / window_size according to PEP 8 style guidelines.
| result=total/window_size | |
| result = total / window_size |
| # pass | ||
| list=[] | ||
| for i in list: | ||
| if i>threshold: |
There was a problem hiding this comment.
Missing spacing around the comparison operator. Should be if i > threshold: according to PEP 8 style guidelines.
| if i>threshold: | |
| if i > threshold: |
| list=[] | ||
| for i in list: | ||
| if i>threshold: | ||
| return i | ||
| else: | ||
| return list |
There was a problem hiding this comment.
The implementation has multiple critical bugs:
- Variable shadowing: Line 21 uses
listas a variable name, which shadows Python's built-inlisttype. - Iterating over empty list: Line 22 iterates over the newly created empty list, so the loop body never executes.
- Incorrect parameter usage: The function should iterate over the
salesparameter, not the locallistvariable. - Premature return: Lines 24 and 26 return immediately on the first iteration, preventing the function from collecting all qualifying values.
- Wrong return type: The function returns individual integers instead of a list of all qualifying values.
The correct implementation should iterate over sales, collect all values greater than threshold in a result list, and return that list after checking all values.
| list=[] | |
| for i in list: | |
| if i>threshold: | |
| return i | |
| else: | |
| return list | |
| result = [] | |
| for sale in sales: | |
| if sale > threshold: | |
| result.append(sale) | |
| return result |
No description provided.