Skip to content
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

New functions : Vector2.angle and Vector2.angle_rad and the tests and documentation needed #3216

Closed
wants to merge 6 commits into from
Closed

New functions : Vector2.angle and Vector2.angle_rad and the tests and documentation needed #3216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1932b82
Changes to be commited:
Oct 16, 2024
49dbee5
Delete test.txt
GabrieleLS Oct 16, 2024
969cdce
Fix functions in rect_test to use literal values for attribute checks
AntoineMamou Oct 23, 2024
1f69517
Add two functions : Vector2.angle and Vector2.angle_rad and the tests…
AntoineMamou Nov 7, 2024
e0d7289
rect_test modification
AntoineMamou Nov 11, 2024
9eb4b61
Merge branch 'pygame-community:main' into vector2Angle
AntoineMamou Nov 12, 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
2 changes: 2 additions & 0 deletions src_c/doc/math_doc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
#define DOC_MATH_VECTOR2_ANGLETO "angle_to(Vector2, /) -> float\ncalculates the angle to a given vector in degrees."
#define DOC_MATH_VECTOR2_ASPOLAR "as_polar() -> (r, phi)\nreturns a tuple with radial distance and azimuthal angle."
#define DOC_MATH_VECTOR2_FROMPOLAR "from_polar((r, phi), /) -> None\nSets x and y from a polar coordinates tuple."
#define DOC_MATH_VECTOR2_ANGLERAD "angle_rad() -> float\nreturns the angle of the vector in radians relative to the positive X-axis."
Copy link
Member

Choose a reason for hiding this comment

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

I see that this macros have been created, but the doc RST file has not been edited, making me believe you added them manually. To add a new feature you should edit the math.rst found somewhere under the docs folder and when you added proper documentation run the command py dev.py docs (or py -m buildconfig docs) that will generate them automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I'm new so I didn't know that. I'm going to fix that. But I don't understand what this macros is doing if it's not to create a doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However, with @oddbookworm comment. Do you think I should update my code and make a new PR for this issue or not ?

Copy link
Member

Choose a reason for hiding this comment

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

@AntoineMamou for the docs header, did you not read the first line of the file? Where it says it's autogenerated from the actual docs?

Copy link
Member

Choose a reason for hiding this comment

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

Which part of my comment do you want to push off to another PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I didn't see it when I make the PR

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 was just thinking about the attributes angle and angle_rad

#define DOC_MAT_VECTOR2_ANGLE "angle_rad() -> float\nreturns the angle of the vector in degrees relative to the positive X-axis, within interval (-180, 180]."
#define DOC_MATH_VECTOR2_PROJECT "project(Vector2, /) -> Vector2\nprojects a vector onto another."
#define DOC_MATH_VECTOR2_COPY "copy() -> Vector2\nReturns a copy of itself."
#define DOC_MATH_VECTOR2_CLAMPMAGNITUDE "clamp_magnitude(max_length, /) -> Vector2\nclamp_magnitude(min_length, max_length, /) -> Vector2\nReturns a copy of a vector with the magnitude clamped between max_length and min_length."
Expand Down
27 changes: 27 additions & 0 deletions src_c/math.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2499,6 +2499,30 @@ vector2_from_polar(pgVector *self, PyObject *args)

Py_RETURN_NONE;
}

static PyObject *
vector2_angle_rad(pgVector *self, PyObject *_null)
{
double angle_rad;
angle_rad = atan2(self->coords[1], self->coords[0]);
return PyFloat_FromDouble(angle_rad);
}

static PyObject *
vector2_angle(pgVector *self, PyObject *_null)
{
double angle_rad = atan2(self->coords[1], self->coords[0]);
double angle_deg = RAD2DEG(angle_rad);

if (angle_deg > 180) {
angle_deg -= 360;
}
else if (angle_deg <= -180) {
angle_deg += 360;
}
return Py_BuildValue("d", angle_deg);
Copy link
Member

Choose a reason for hiding this comment

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

Why have you used Py_BuildValue here and PyFloat_FromDouble in the other function? I feel like they should both use the latter. also, I feel like there are a few unnecessary declarations that could be avoided reducing a bit the lines, if that doesn't decrease readability too much (but formatting the code might bring it back)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you are right. Because I begin in this project, I saw that there is these two function and I try it both. Since it works, I didn't see the problem but if both should use PyFloat_FromDouble, it's ok.

}

static PyObject *
vector_getsafepickle(pgRectObject *self, void *_null)
{
Expand Down Expand Up @@ -2566,6 +2590,9 @@ static PyMethodDef vector2_methods[] = {
DOC_MATH_VECTOR2_ASPOLAR},
{"from_polar", (PyCFunction)vector2_from_polar, METH_VARARGS,
DOC_MATH_VECTOR2_FROMPOLAR},
{"angle_rad", (PyCFunction)vector2_angle_rad, METH_NOARGS,
DOC_MATH_VECTOR2_ANGLERAD},
{"angle", (PyCFunction)vector2_angle, METH_NOARGS, DOC_MATH_VECTOR2_ANGLE},
{"project", (PyCFunction)vector2_project, METH_O,
DOC_MATH_VECTOR2_PROJECT},
{"copy", (PyCFunction)vector_copy, METH_NOARGS, DOC_MATH_VECTOR2_COPY},
Expand Down
71 changes: 71 additions & 0 deletions test/math_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,77 @@ def test_polar(self):
v.from_polar((1, 0))
self.assertEqual(v, self.e1)

def test_angle_rad(self):
v1 = Vector2(1, 0)
self.assertEqual(v1.angle_rad(), 0)

v2 = Vector2(0, 1)
self.assertEqual(v2.angle_rad(), math.pi / 2)

v3 = Vector2(-1, 0)
self.assertEqual(v3.angle_rad(), math.pi)

v4 = Vector2(1, 1)
self.assertEqual(v4.angle_rad(), math.pi / 4)

v5 = pygame.math.Vector2(0, 0)
self.assertEqual(v5.angle_rad(), 0)

v6 = pygame.math.Vector2(1, 1e-6)
self.assertAlmostEqual(v6.angle_rad(), 0, places=5)

v7 = pygame.math.Vector2(1e6, 1e6)
self.assertAlmostEqual(v7.angle_rad(), math.pi / 4, places=5)

v8 = pygame.math.Vector2(1, -1)
self.assertEqual(v8.angle_rad(), -math.pi / 4)

v9 = pygame.math.Vector2(-1, 1)
self.assertEqual(v9.angle_rad(), 3 * math.pi / 4)

v10 = pygame.math.Vector2(-1, -1)
self.assertEqual(v10.angle_rad(), -3 * math.pi / 4)

v11 = pygame.math.Vector2(1, 1)
self.assertEqual(v11.angle_rad(), math.pi / 4)

v12 = pygame.math.Vector2(1e-6, 1)
self.assertAlmostEqual(v12.angle_rad(), math.pi / 2, places=5)

def test_angle(self):
v1 = Vector2(0, 0)
self.assertTrue(v1.angle() == 0)

v2 = Vector2(1, 0)
self.assertTrue(v2.angle() == 0)

v3 = Vector2(1, 1)
self.assertTrue(v3.angle() == 45)

v4 = Vector2(0, 1)
self.assertTrue(v4.angle() == 90)

v5 = Vector2(-1, 1)
self.assertTrue(v5.angle() == 135)

v6 = Vector2(-1, 0)
self.assertTrue(v6.angle() == 180)

v7 = Vector2(-1, -1)
self.assertTrue(v7.angle() == -135)

v8 = Vector2(0, -1)
self.assertTrue(v8.angle() == -90)

v9 = Vector2(1, -1)
self.assertTrue(v9.angle() == -45)

v10 = Vector2(-0.005235964, 0.9999863)
self.assertAlmostEqual(v10.angle(), 90.3, places=5)

v11 = Vector2(math.cos(math.radians(-20)), math.sin(math.radians(-20)))
self.assertAlmostEqual(v11.angle(), -20, places=5)

def test_subclass_operation(self):
class Vector(pygame.math.Vector2):
pass
Expand Down
56 changes: 28 additions & 28 deletions test/rect_test.py
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these changes relevant to this pr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello,

No, not at all. This was a mistake I made because I was working on an other issue at that time. By the way, this PR should be close I think because it's this PR that is link to the issue of .angle and .angle_rad : #3222

Original file line number Diff line number Diff line change
Expand Up @@ -777,27 +777,27 @@ def test_inflate__larger(self):
r = Rect(2, 4, 6, 8)
r2 = r.inflate(4, 6)

self.assertEqual(r.center, r2.center)
self.assertEqual(r.left - 2, r2.left)
self.assertEqual(r.top - 3, r2.top)
self.assertEqual(r.right + 2, r2.right)
self.assertEqual(r.bottom + 3, r2.bottom)
self.assertEqual(r.width + 4, r2.width)
self.assertEqual(r.height + 6, r2.height)
self.assertEqual(r2.center, (5, 8))
self.assertEqual(r2.left, 0)
self.assertEqual(r2.top, 1)
self.assertEqual(r2.right, 10)
self.assertEqual(r2.bottom, 15)
self.assertEqual(r2.width, 10)
self.assertEqual(r2.height, 14)

def test_inflate__smaller(self):
"""Ensures deflating a rect keeps its center the same
and shrinks dimensions by correct values."""
r = Rect(2, 4, 6, 8)
r2 = r.inflate(-4, -6)

self.assertEqual(r.center, r2.center)
self.assertEqual(r.left + 2, r2.left)
self.assertEqual(r.top + 3, r2.top)
self.assertEqual(r.right - 2, r2.right)
self.assertEqual(r.bottom - 3, r2.bottom)
self.assertEqual(r.width - 4, r2.width)
self.assertEqual(r.height - 6, r2.height)
self.assertEqual((5, 8), r2.center)
self.assertEqual(4, r2.left)
self.assertEqual(7, r2.top)
self.assertEqual(6, r2.right)
self.assertEqual(9, r2.bottom)
self.assertEqual(2, r2.width)
self.assertEqual(2, r2.height)

def test_inflate_ip__larger(self):
"""Ensures inflating a rect in place keeps its center the same
Expand All @@ -806,13 +806,13 @@ def test_inflate_ip__larger(self):
r2 = Rect(r)
r2.inflate_ip(4, 6)

self.assertEqual(r.center, r2.center)
self.assertEqual(r.left - 2, r2.left)
self.assertEqual(r.top - 3, r2.top)
self.assertEqual(r.right + 2, r2.right)
self.assertEqual(r.bottom + 3, r2.bottom)
self.assertEqual(r.width + 4, r2.width)
self.assertEqual(r.height + 6, r2.height)
self.assertEqual(r2.center, (5, 8))
self.assertEqual(r2.left, 0)
self.assertEqual(r2.top, 1)
self.assertEqual(r2.right, 10)
self.assertEqual(r2.bottom, 15)
self.assertEqual(r2.width, 10)
self.assertEqual(r2.height, 14)

def test_inflate_ip__smaller(self):
"""Ensures deflating a rect in place keeps its center the same
Expand All @@ -821,13 +821,13 @@ def test_inflate_ip__smaller(self):
r2 = Rect(r)
r2.inflate_ip(-4, -6)

self.assertEqual(r.center, r2.center)
self.assertEqual(r.left + 2, r2.left)
self.assertEqual(r.top + 3, r2.top)
self.assertEqual(r.right - 2, r2.right)
self.assertEqual(r.bottom - 3, r2.bottom)
self.assertEqual(r.width - 4, r2.width)
self.assertEqual(r.height - 6, r2.height)
self.assertEqual(r2.center, (5, 8))
self.assertEqual(r2.left, 4)
self.assertEqual(r2.top, 7)
self.assertEqual(r2.right, 6)
self.assertEqual(r2.bottom, 9)
self.assertEqual(r2.width, 2)
self.assertEqual(r2.height, 2)

def test_scale_by__larger_single_argument(self):
"""The scale method scales around the center of the rectangle"""
Expand Down
Loading