Add infinity and figure eight target shapes #190

MauriceDeVr · 2024-07-13T12:35:49Z

Fixes: #179
Fixes: #178

Changes

Added infinity Target Shape
Added Figure Eight Shape

github-actions

Congratulations on making your first pull request to Data Morph! Please familiarize yourself with the contributing guidelines, if you haven't already.

codecov · 2024-07-16T23:07:57Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.44%. Comparing base (51b8d26) to head (0853d0a).
Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #190      +/-   ##
==========================================
+ Coverage   98.42%   98.44%   +0.01%     
==========================================
  Files          43       43              
  Lines        1775     1795      +20     
  Branches      358      356       -2     
==========================================
+ Hits         1747     1767      +20     
  Misses         25       25              
  Partials        3        3

Files	Coverage Δ
src/data_morph/shapes/factory.py	`100.00% <ø> (ø)`
src/data_morph/shapes/points.py	`100.00% <100.00%> (ø)`
tests/shapes/test_points.py	`100.00% <100.00%> (ø)`

stefmolin

Thanks for the PR, @MauriceDeVr! We are off to a good start.

stefmolin · 2024-07-16T23:07:51Z

src/data_morph/shapes/points.py

+    def __str__(self) -> str:
+        return 'infinity'
+


We only need this if the name is different than the lowercased class name.

Suggested change

def __str__(self) -> str:

return 'infinity'

data-morph/src/data_morph/shapes/bases/shape.py

Lines 25 to 34 in b6bebc9

def __str__(self) -> str:

"""

Return string representation of the shape.

Returns

-------

str

The human-readable string representation of the shape.

"""

return self.__class__.__name__.lower()

stefmolin · 2024-07-16T23:08:41Z

tests/shapes/test_points.py

+        # print(shape.points[67])
+        print(shape.distance(*test_point))


Suggested change

# print(shape.points[67])

print(shape.distance(*test_point))

stefmolin · 2024-07-16T23:10:59Z

src/data_morph/shapes/points.py

+    Notes
+    -----
+    The formula for the infinity shape is directly taken from Lemniscate of
+    Bernoulli equation.


This feels more like a see also section. Something like this, but figure out a good description:

Suggested change

Notes

-----

The formula for the infinity shape is directly taken from Lemniscate of

Bernoulli equation.

See Also

--------

_LemniscateBernoulli : Write a description here

stefmolin · 2024-07-16T23:11:31Z

src/data_morph/shapes/points.py

+    Notes
+    -----
+    Implements the Lemniscate of Bernoulli Equation with a transform
+    to draw a figure eight shape.
+
+    See Base class for implementation specifice details.


Let's do a see also section here as well, but also link to the Infinity class.

stefmolin · 2024-07-16T23:17:03Z

src/data_morph/shapes/points.py

+        super().__init__(
+            *np.stack(
+                [
+                    transformed_x * scale_factor + x_shift,
+                    transformed_y * scale_factor + y_shift,
+                ],
+                axis=1,
+            )
+        )


The shapes both have a gap. Let's make it a closed shape:

stefmolin · 2024-07-16T23:18:05Z

src/data_morph/shapes/points.py

+        x_shift = sum(x_bounds) / 2
+        y_shift = sum(y_bounds) / 2
+
+        t = np.linspace(-3, 3, num=2000)


Using 2,000 points is a lot, the algorithm needs to calculate the distance to each point. Can we reduce this? For example, the heart only uses 80.

stefmolin · 2024-07-16T23:21:09Z

src/data_morph/shapes/points.py

@@ -131,6 +132,150 @@ def __init__(self, dataset: Dataset) -> None:
        )


+class _LemniscateBernoulli(PointCollection):


Let's merge this with the Infinity class (calling it Infinity). The docs have a broken link with this because they ignore anything prefixed with _, and this class has all the functionality of the Infinity class already.

stefmolin · 2024-07-16T23:37:11Z

src/data_morph/shapes/points.py

+        x = (np.sqrt(2) * np.cos(t)) / (1 + np.square(np.sin(t)))
+        y = (np.sqrt(2) * np.cos(t) * np.sin(t)) / (1 + np.square(np.sin(t)))
+
+        scale_factor = (x_bounds[1] - x_shift) * 0.75


Why not use the y information here? Maybe pick one using the data? With the sheep for example, it seems like the figure eight is being drawn too tall, so we should probably take the minimum of the x and y scale factors and use that.

Try plotting a dataset and the shape on top of each other and see how that looks.

Your Name added 2 commits July 13, 2024 13:32

Add Infinity Target Shape

98ebfac

Update Infinity documentation

92683cc

github-actions bot added the shapes Work relating to shapes module label Jul 13, 2024

github-actions bot reviewed Jul 13, 2024

View reviewed changes

stefmolin added this to the 0.3.0 milestone Jul 13, 2024

Your Name added 2 commits July 13, 2024 14:40

Implement Figure Eight

9d51b10

Add tests for Infinity and Figure Eight shapes

82f870e

github-actions bot added the testing Relating to the testing suite label Jul 13, 2024

Your Name and others added 2 commits July 13, 2024 15:06

Make precommit changes

dbb735c

Merge branch 'main' into Add-infinity-target-shape-stefmolin#179

0853d0a

stefmolin changed the title ~~Add infinity target shape #179~~ Add infinity and figure eight target shapes Jul 16, 2024

stefmolin requested changes Jul 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add infinity and figure eight target shapes #190

Add infinity and figure eight target shapes #190

MauriceDeVr commented Jul 13, 2024 •

edited

Loading

github-actions bot left a comment

codecov bot commented Jul 16, 2024 •

edited

Loading

stefmolin left a comment

stefmolin Jul 16, 2024

stefmolin Jul 16, 2024

stefmolin Jul 16, 2024

stefmolin Jul 16, 2024

stefmolin Jul 16, 2024

stefmolin Jul 16, 2024

stefmolin Jul 16, 2024

stefmolin Jul 16, 2024

	def __str__(self) -> str:
	"""
	Return string representation of the shape.

	Returns
	-------
	str
	The human-readable string representation of the shape.
	"""
	return self.__class__.__name__.lower()

		@@ -131,6 +132,150 @@ def __init__(self, dataset: Dataset) -> None:
		)


		class _LemniscateBernoulli(PointCollection):

Add infinity and figure eight target shapes #190

Are you sure you want to change the base?

Add infinity and figure eight target shapes #190

Conversation

MauriceDeVr commented Jul 13, 2024 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 16, 2024 • edited Loading

Codecov Report

stefmolin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MauriceDeVr commented Jul 13, 2024 •

edited

Loading

codecov bot commented Jul 16, 2024 •

edited

Loading