Fixed changes, completely done program - #3788
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the core requirement is not met and CI is failing. The task asks for sorted_variables to be explicitly defined with the 8 given variables directly assigned to the "mutable" and "immutable" lists, but your current app/main.py builds the dictionary at runtime via a function and starts with empty lists, which violates the specification. Please replace the runtime population with a direct assignment like sorted_variables = {"mutable": [my_favourite_films, marks, collection_of_coins], "immutable": [lucky_number, pi, one_is_a_prime_number, name, profile_info]} and remove the unnecessary function and calls.
Additionally, flake8-annotations reports missing type hints at line 25: ANN001 (missing type annotation for argument value) and ANN201 (missing return type). Add parameter and return type annotations (e.g., def your_function(value: <Type>) -> <ReturnType>: or -> None if it returns nothing) to satisfy the linter. Minor note: the GitHub Actions warnings about Node versions are non-blocking, but updating to newer action versions will silence them. Once you make these changes, the implementation should align with the requirements and pass CI. Keep going—your variable setup looks close, it just needs to be wired directly into the dictionary as required.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| def find_value_type(value): | ||
| if type(value) in (str, float, int, bool, tuple): | ||
| sorted_variables["immutable"].append(value) | ||
| elif value is None: | ||
| sorted_variables["immutable"].append(value) | ||
| if type(value) in (list, dict, set): | ||
| sorted_variables["mutable"].append(value) | ||
|
|
||
|
|
||
| find_value_type(lucky_number) | ||
| find_value_type(pi) | ||
| find_value_type(one_is_a_prime_number) | ||
| find_value_type(name) | ||
| find_value_type(my_favourite_films) | ||
| find_value_type(profile_info) | ||
| find_value_type(marks) | ||
| find_value_type(collection_of_coins) |
There was a problem hiding this comment.
The entire function and all function calls are unnecessary. The task requires direct assignment of variables to the dictionary lists, not runtime population through function calls.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Here is my approval: Your implementation in app/main.py correctly defines the sorted_variables dictionary with all required mutable and immutable types, and it satisfies the task’s functional requirements. The previous runtime population issue is resolved by directly defining the dictionary, which is clear and reliable.
Minor note: there’s a linting issue flagged by flake8: W391 at app/main.py:24:1 (blank line at end of file). While this doesn’t affect functionality, it blocks CI. To fix it, remove the extra trailing blank line so the file ends right after the last line of code (keeping only the standard single terminating newline), then re-run flake8 app/ to confirm it’s clean.
Overall, nice work organizing the variables and meeting the requirements. Keep up the good structure and attention to detail!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved! This is an educational override to help you progress while learning. You correctly sorted all 8 variables into the right categories — the sorted_variables dictionary structure is exactly as required. Great job on understanding mutability in Python! There's just one small fix needed: remove the trailing blank line at line 24 in app/main.py so the file ends with the closing } and a standard newline. This will clear the flake8 W391 linting error and allow your CI to pass. After making this change, run flake8 app/ to verify everything is clean — [CHECKLIST ITEMS #1-2]. Keep up the excellent work!
Tip: Python style guides (PEP 8) recommend files end with exactly one newline character and no additional blank lines.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.