Library: Point class

Question

Recently I made a point dataclass and a point_calc class which performs various commonly used algorithms on a point.

from dataclasses import dataclass
import math
EPS = 7./3 - 4./3 -1
#note 7./3 - 4./3 -1: https://stackoverflow.com/questions/19141432/python-numpy-machine-epsilon
@dataclass
class point:
 
 x: float = 0.0
 y: float = 0.0
 #x first then y (2,3) > (1,4) and (2,4) > (2,3)
 
 def __lt__(self, other) -> bool:
 if (abs(self.x - other.x) > EPS):
 return self.x < other.x
 return self.y < other.y
 def __eq__(self, other) -> bool:
 return (abs(self.x - other.x) < EPS) and (abs(self.y - other.y) < EPS)
 def __str__(self):
 return "("+str(point.x)+" ,"+str(point.y)+")"
 
class point_calc:
 def dist(self, point, other, t) -> float: # t=0=euclidean, t=1=manhattan
 match t:
 case 0:
 return math.dist([point.x, point.y], [other.x, other.y])
 case 1:
 return float(abs(abs(point.x-other.x)+abs(point.y-other.y)))
 case _:
 raise Exception("No Such Function")
 ###### HELPER FUNCS ######
 def dr(self, d) -> float:
 return (d*math.pi)/180
 def rd(self, r) -> float:
 return (r*180)/math.pi
 # assumes theta is in Degrees, if not, just use point_clac.rd() to convert to deg
 def rotate(self, point, theta, accuracy = 2) -> tuple:
 dr = self.dr
 return (round(point.x*math.cos(dr(theta))-point.y*math.sin(dr(theta)), accuracy),
 round(point.x*math.sin(dr(theta))+point.y*math.cos(dr(theta)), accuracy))

Is there any suggestions on how it can be improved? Any suggestions will be accepted and any help (or constructive criticism) will be appreciated.

• Trang Oul: Consider using turtle.Vec2D.

Answer 1

Style

You should follow PEP 8 style guide, which would help following along your code when reading it, whether it should be reused by someone else, reviewed, or revisited by yourself in the future, when you are more familiar with Python's conventions.

Things that should be improved here:

• Surround top-level imports, classes and function defs with 2 blank lines
• Do not leave blank lines between comments and the thing they refer to
• Prefer docstrings over comments for documenting functions and classes (see also PEP 257 - Docstring Conventions)
• Capitalize your class names

Epsilon

The way you get the float epsilon value feels hacky and isn't immediately clear to what it does. Prefer using sys.float_info.epsilon.

"Static" class

Your point_calc class behaves like a static class in other languages. Such a concept doesn't really exist in Python, and top-level functions would be preferred for this kind of functionality.

Naming

Some of your functions and variable names can use more descriptive names:

• rd, dr and t are cryptic and not understandable without understanding the code inside: prefer degrees_to_radians, radians_to_degrees and distance_type
• dist and point_calc are needlessly abbreviated: prefer distance and point_calculations
• theta (argument of rotate) requires knowledge of some mathematical conventions: prefer angle, or better yet angle_degrees or simply degrees to alleviate the ambiguity over units
• rotate is ambiguous: prefer rotate_around_origin

Don't reinvent the wheel

The math standard library module provide the degrees and radians functions, that you should use instead of rolling your own angle conversion, resulting in shorter code.

Single responsibility

Your dist function can do two different things: calculate the Euclidean distance or the Manhattan distance between two points. Although these concepts are related, you either use one or the other, so make it two different function: euclidean_distance(point_a, point_b) and manhattan_distance(point_a, point_b).

The result will be more readable, both by giving each function a single responsibility, and by eliminating the need for a third argument. The third argument would be especially hard to parse for a human reader at the call site.

In cases where you need such an argument for specifying an option, at least use named constants or an enum to pass to the function, along with a meaningful argument name and complete documentation. I don't believe it applies here, as having separate functions is shorter and more readable.

Exception choice

When you raise an exception, avoid using a generic Exception and use as specific an exception as you can, making error handling easier to use down the line. In your case, a ValueError is probably the best fit for invalid values of t in the dist function (although it can be avoided altogether, see above).

Consistency

Your point class implementation of __lt__ compares distances along the x-axis to EPS but not along the y-axis.

However, the implementation of __eq__ compare the distance along both axes to EPS.

I personally wouldn't compare to EPS in either of these methods, and add a close_to method to the point class: def close_to(self, other, tolerance=EPS).

• Ray: sys.float_info.epsilon isn't necessarily the epsilon value you want here. That's the "difference between 1.0 and the least value greater than 1.0 that is representable as a float", not the minimal difference before the numbers have different representations. The "proper" epsilon value will depend on the scale of the numbers you're working with. Read What Every Computer Scientist Should Know About Floating-Point Arithmetic to understand what's happening in sufficient detail. "Units in Last Place" is the key concept.
• gazoh: True, I didn't get in the details of floating point comparison, but I feel like it's at least partially addressed in the last suggestion in my answer. Furthermore, sys.float_info.epsilon does resolve to the same value than 7/3 - 4/3 - 1, so if that's what the person asking wants, fine, but let's at least define that value in a readable way.
• Trang Oul: Speaking of reinventing the wheel, the standard library contains turtle.Vec2D. It even supports both degrees (default) or radians (or any other unit) for rotations.

Answer 2

i am not a pythonist so I won't comment on the code itself. I just want to point out one nitpicky formalism.

What does it mean to rotate a point? Are you using the structure to represent a vector as well as point? You can only rotate a vector, or you can rotate a point around another point. Rotating a point around origin is the same as rotating vector. Your rotate method is either a method of vector in which case it is confusing why it is a method of point. Or it is a special case of rotation of one point around another one in which case it has unnecessary lack of generality. Either give that method an extra parameter of type point and it can have default value of the origin 0,0 or rename the method to rotate_around_origin.

Anyway I would suggest to move that method directly to the point class, there is no reason to put it aside.

On other hand the rest of the point_calc class could be renamed to angle_unit_convertor because the moment you removed the rotate method, angle conversions is all there is left and it's not generally coupled to points anyway, you'll find angle conversions useful in other contexts too.

Also you should not force the consumer to rotate in degrees. Rotate in the unit that is natural to the computation - that is in radians. Let consumer convert units as they see fit.

p.rotate_around_origin(pi/2)
p.rotate_around_origin(deg2rad(90))

Similarly for the rounding. Why is accuracy a parameter to rotation? Let consumer do whatever rounding they want when they want. Provide a round method that consumer can run to round both components of the point using the same accuracy.

p.rotate(angle, origin).round(accuracy)

Answer 3

A small addition to the existing reviews. Consider making the dataclass read-only. This means that points cannot be edited, and functions making operations on points will have to return a new point. The benefit of this is that you can then configure the dataclass to generate a __hash__ method, allowing your points to be used as keys in dictionaries or sets. This is potentially useful if you're working with collections of points in a grid.

The way you would do this is to add the following parameters to your decorator:

@dataclass(eq=True, frozen=True)

eq=True would usually generate an equality method, but since you're defining your own, it's ignored. It needs to be True along with frozen=True in order for dataclass to generate your __hash__ method.