Skip to content

Conversation

@CookieTanuki
Copy link

No description provided.

app/main.py Outdated
def __repr__(self) -> str:
return f"Distance(km={self.km})"

def __add__(self, other: object) -> "Distance":
Copy link

Choose a reason for hiding this comment

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

Change type hint "object" to "Distance | int | float"

Copy link

@vakt159 vakt159 left a comment

Choose a reason for hiding this comment

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

Everything is good, just change type hints

app/main.py Outdated
other = other.km
return Distance(self.km + other)

def __iadd__(self, other: object) -> "Distance":
Copy link

Choose a reason for hiding this comment

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

Same thing here

app/main.py Outdated
self.km += other
return self

def __mul__(self, other: float) -> "Distance":
Copy link

Choose a reason for hiding this comment

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

other can be int and float

app/main.py Outdated
def __mul__(self, other: float) -> "Distance":
return Distance(self.km * other)

def __truediv__(self, other: float) -> "Distance":
Copy link

Choose a reason for hiding this comment

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

other can be int and float

app/main.py Outdated
def __truediv__(self, other: float) -> "Distance":
return Distance(round(self.km / other, 2))

def __lt__(self, other: object) -> bool:
Copy link

Choose a reason for hiding this comment

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

Change type hint "object" to "Distance | int | float"

app/main.py Outdated
other = other.km
return self.km < other

def __gt__(self, other: object) -> bool:
Copy link

Choose a reason for hiding this comment

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

same here

app/main.py Outdated
other = other.km
return self.km > other

def __eq__(self, other: object) -> bool:
Copy link

Choose a reason for hiding this comment

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

same here

app/main.py Outdated
other = other.km
return self.km == other

def __le__(self, other: object) -> bool:
Copy link

Choose a reason for hiding this comment

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

same here

app/main.py Outdated
other = other.km
return self.km <= other

def __ge__(self, other: object) -> bool:
Copy link

Choose a reason for hiding this comment

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

same here

Copy link

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

Choose a reason for hiding this comment

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

Review Summary

Great work on implementing the core logic for the Distance class! All the required methods are present and function correctly. However, I'm requesting changes because the submission does not meet the type annotation requirements specified in the project checklist.

Specifically, please address the following:

  1. Missing Import: Add from __future__ import annotations to the top of app/main.py. This is required for using modern type hints.
  2. Incorrect Parameter Types: The other parameter in methods like __add__, __iadd__, and all comparison methods should be typed as Distance | int | float instead of the general object.
  3. Incomplete Parameter Types: For __mul__ and __truediv__, the other parameter's type hint should be int | float to correctly reflect that it accepts both integers and floats.

These changes are crucial for adhering to the project's coding standards for type safety. I look forward to seeing your updated submission!


✨ 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

app/main.py Outdated
def __repr__(self) -> str:
return f"Distance(km={self.km})"

def __add__(self, other: object) -> "Distance":

Choose a reason for hiding this comment

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

The type hint for other should be more specific. According to the checklist, you should use Distance | int | float instead of object. Additionally, the return type should be Distance without quotes, which requires adding from __future__ import annotations at the top of the file. This violates checklist items under 'Type Annotations'.

app/main.py Outdated
other = other.km
return Distance(self.km + other)

def __iadd__(self, other: object) -> "Distance":

Choose a reason for hiding this comment

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

Similar to __add__, the type hint for other should be Distance | int | float and the return type should be Distance (without quotes). Please ensure you add from __future__ import annotations to the file.

app/main.py Outdated
self.km += other
return self

def __mul__(self, other: float) -> "Distance":

Choose a reason for hiding this comment

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

This method should support multiplication by both integers and floats. The type hint for other should be int | float. This violates checklist item #1 under 'Type Annotations'.

app/main.py Outdated
def __mul__(self, other: float) -> "Distance":
return Distance(self.km * other)

def __truediv__(self, other: float) -> "Distance":

Choose a reason for hiding this comment

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

Similar to __mul__, this method should also support division by both integers and floats. The type hint for other should be int | float.

app/main.py Outdated
def __truediv__(self, other: float) -> "Distance":
return Distance(round(self.km / other, 2))

def __lt__(self, other: object) -> bool:

Choose a reason for hiding this comment

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

The type hint for other should be Distance | int | float instead of object for better type safety and clarity. This applies to all the comparison methods (__gt__, __eq__, __le__, __ge__).

Copy link

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

Choose a reason for hiding this comment

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

Review Summary

Excellent work on this revision! I am approving your solution as you have successfully addressed all the feedback from the previous review. The Distance class is now fully implemented, all methods work as required, and the type annotations are correctly applied. The code meets all the requirements of the task. Great job!


✨ 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.

3 participants