Skip to content

Solution#2964

Open
Woland32 wants to merge 3 commits into
mate-academy:masterfrom
Woland32:develop
Open

Solution#2964
Woland32 wants to merge 3 commits into
mate-academy:masterfrom
Woland32:develop

Conversation

@Woland32

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because there are still two critical requirement violations that affect both functionality and the explicit checklist. First, for inputs where wife (and by symmetry husband) is None, your code currently creates person.wife = None and person.husband = None; this means accessing person.wife will return None instead of raising AttributeError as the task explicitly requires. You should only create these attributes when the corresponding input value is not None, so that missing spouses truly have no attribute.

Second, building the result list with a traditional for loop violates the explicit requirement to use a list comprehension when creating a list of instances from a collection [CHECKLIST ITEM #3]. Refactor the construction of result to something like result = [Person(p["name"], p["age"]) for p in people] (adapting to your actual signature) while preserving the spouse-linking logic afterward. Aside from these points, your class design, use of Person.people, and spouse wiring for non-None values look good. Once you fix the conditional spouse attributes and switch to a list comprehension for instance creation, your solution should fully meet the task requirements and tests.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread app/main.py Outdated
Comment on lines +21 to +22
else:
person.wife = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sets person.wife = None when wife is None in the dict, so the wife attribute always exists and will not raise AttributeError as required. This violates the description requirement that for wife equal to None, accessing .wife should raise AttributeError. Consider not creating the wife attribute at all in this branch.

Comment thread app/main.py Outdated
Comment on lines +25 to +26
else:
person.husband = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, this sets person.husband = None when husband is None, so the attribute is always present. While the description only explicitly mentions wife, symmetry suggests you should avoid creating the husband attribute when there is no husband in the input.

Comment thread app/main.py Outdated
Comment on lines +12 to +15
result = []
for person_data in people:
person = Person(person_data["name"], person_data["age"])
result.append(person)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building result with a traditional for loop violates checklist item #3: When creating a list of instances from a collection (such as a list of dictionaries), it is recommended to use list comprehension rather than traditional for loops. Consider using a list comprehension to create the list of Person instances from people.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because one explicit checklist requirement is still not met: the creation of the result list in create_person_list must use a list comprehension instead of a traditional for loop. Functionally, your implementation of Person, the people class attribute, and the spouse wiring logic all look correct and match the required behavior, including only setting spouse attributes when wife/husband is not None. However, the task’s checklist clearly states that when building a list of instances from a collection, a list comprehension must be used, so this is treated as a critical requirement rather than a stylistic preference.

The specific change you need is in app/main.py inside create_person_list, where you currently do something like:

  • result = [] followed by a for person_data in people: loop that appends new Person instances. Instead, construct result with a single expression, for example: result = [Person(p["name"], p["age"]) for p in people], and then keep your second pass that wires spouses as it is. Once you refactor this part to a list comprehension, your solution will both satisfy [CHECKLIST ITEM #3] and maintain the correct behavior you already have. Overall, you’re very close—this is a small structural change that will bring your code fully in line with the requirements.

✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread app/main.py Outdated
def create_person_list(people: list) -> list:
# write your code here
pass
result = []

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item #3: When creating a list of instances from a collection (such as a list of dictionaries), it is recommended to use list comprehension rather than traditional for loops. Refactor the creation of result so the Person instances are built with a list comprehension instead of an explicit for loop.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am approving your solution because it fully meets the functional requirements and follows the checklist and style expectations. Your Person class correctly defines name and age, maintains the people dictionary as a class attribute, and registers each instance in __init__, which shows a solid understanding of class-level state. In create_person_list, you correctly build a new list of Person objects from the input dictionaries using a list comprehension, satisfying [CHECKLIST ITEM #3] while keeping the code clear and concise. The spouse relationships are wired properly using person_data.get("wife") and person_data.get("husband"), which respects [CHECKLIST ITEM #4] and avoids unnecessary key checks. The example behaviors—correct types, names, spouse links, AttributeError when a spouse attribute doesn’t exist, and the final contents of Person.people—will all behave as expected with your implementation. Overall, this is clean, idiomatic Python code that demonstrates good design decisions and attention to the requirements—well done and keep building on this approach in future exercises.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants