Skip to content

compare-method added to Vector class in lib.py #12448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 28, 2024
Merged

compare-method added to Vector class in lib.py #12448

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5c5fcf2
compare-method added to Vector class in lib.py
Andrwaa Dec 22, 2024
5eb2483
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 22, 2024
aeca5c1
Updated lib.py with suggestions
Andrwaa Dec 28, 2024
4816439
Updated lib.py with suggestions
Andrwaa Dec 28, 2024
bbeeb18
Updated lib.py with __eq__ method
Andrwaa Dec 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion linear_algebra/src/lib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ class Vector:
change_component(pos: int, value: float): changes specified component
euclidean_length(): returns the euclidean length of the vector
angle(other: Vector, deg: bool): returns the angle between two vectors
TODO: compare-operator
"""

def __init__(self, components: Collection[float] | None = None) -> None:
Expand Down Expand Up @@ -183,6 +182,16 @@ def angle(self, other: Vector, deg: bool = False) -> float:
else:
return math.acos(num / den)

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

Choose a reason for hiding this comment

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

Could you move the method higher up in the file, before all the public methods? Having this one dunder method within all the public methods, rather than having it alongside all the other dunder methods, makes it harder to find.

"""
performs the comparison between two vectors
"""
if not isinstance(vector, Vector):
return NotImplemented
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def __eq__(self, vector: object) -> bool:
"""
performs the comparison between two vectors
"""
if not isinstance(vector, Vector):
return NotImplemented
def __eq__(self, vector: Vector) -> bool:
"""
performs the comparison between two vectors
"""

I don't think it makes sense to allow comparison with any object, only with other Vectors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried writing Vector insted of object but it gave me an error that saying: Argument 1 of "eq" is incompatible with supertype "object"; supertype defines the argument type as "object" [override]
This violates the Liskov substitution principle

if len(self) != len(vector):
return False
return all(self.component(i) == vector.component(i) for i in range(len(self)))


def zero_vector(dimension: int) -> Vector:
"""
Expand Down
Loading