• Add tests. There is some code which calls into this but no tests of it directly, and several unreached code paths. (refs #52)
• Add documentation
• Add comments?
• Use vg.major_axis (syntactic sugar for np.linalg.svd)
• Use vg.shape.check?

Link: https://github.com/lace/polliwog/blob/master/polliwog/transform/rodrigues.py

Failing build with numpy 1.19.0:
https://app.circleci.com/pipelines/github/lace/polliwog/836/workflows/b88ac698-51bc-46ea-80b8-f9e57e7157d9/jobs/2618/steps

Passing build with numpy 1.18.4:
https://app.circleci.com/pipelines/github/lace/polliwog/793/workflows/dd32545c-872a-4547-9819-0de088c9f7b8/jobs/2535/steps

Seems a safe bet that we'd want to remove a function with this name.

For the 1.0 release, the minimum standard is at least one sentence of documentation for each function.

Better is to write a full docstring including Args and Returns, with types.

These functions do not yet have a docstring:

• transform.rotation.euler()
• transform.rotation.estimate_normal() (but see #102)
• transform.rodridgues.rodrigues()
• transform.rodrigues.as_rotation_matrix()

This function should be parameterized.

This library is close to being ready for a 1.0 release!

The 1.0 release should include:

• 100% test coverage (#77 #78 #79 #80 #102)
• Hosted documentation (#82)
• At least one sentence of documentation for each function (#134)
• An updated readme

Ideally I'd spend some time polishing the API before releasing 1.0, though I don't want to block on that. I think it's more valuable to release something, and make breaking changes in 2.0, rather than keeping this in 0.x for much longer.

Let's track those issues in https://github.com/lace/polliwog/milestone/1

• Add ToC
• Fix search and index or remove them
• Repair Source links

#71 correctly handles verts on a plane in Polyline.cut_by_plane(), however they are still not handled correctly in Polyline.intersect_plane(). See the expected failing test in test_polyline.py.

Dear Developer,
I am using your package a lot and doing some tests with cloud points and lines.

In my case assume I want calculate the rotation matrix of a cloud of points along a vector starting for a specific point in space for a given angle (at the end is a eurelian rotation around a not-default coordinate system)

In the scatter plot below the rotation is around the center of the cloud (that is not the center of axes as you can see).

But unfortunately to do so I used two transformations.

I was wondering if there is a better way, avoiding two transformations: I didn’t find nothing useful neither in vg.

Thank you for supporting us.

• Add tests (refs #52)
• Remove scipy dependency if at all possible. The goal of this library is to be pure Python + NumPy and it seems like this is possible. Currently it does not declare a scipy dependency.
• Use vg.major_axis (syntactic sugar for np.linalg.svd)
• Use vg.shape.check
• Remove MeshViewer visualization
• Improve readability and add comments

Link: https://github.com/lace/polliwog/blob/master/polliwog/transform/rigid_transform.py

This makes code break now if it's not using the canonical imports, rather than breaking when they're moved down the line.

For a library like this, 100% test coverage ought to be a given.

Here are a few reasons:

1. Tests beget tests – having tests in place ensures that tests get written when code is changed or new paths are added.
2. Tests that cover the edge-case code ensures the code for those edge cases actually works.
3. Once we reach 100% we can also set pytest to enforce it. That way we'll know that all new code is covered without having to dig through coverage reports.

I've made some progress, and coverage is up to 79%!

In the meantime, until the projects reaches 100%, all new code should have 100% coverage.

• Add tests (refs #52)
• Use polliwog.transform.rodrigues in place of cv2.Rodrigues (Is this possible?)
• In estimate_normal, use vg.major_axis (syntactic sugar for np.linalg.svg), removing dependency on sklearn. The goal of this library is to be pure Python + NumPy and it seems like this is possible. Currently it does not declare a scipy dependency.
• Add calls to vg.shape.check

Link: https://github.com/lace/polliwog/blob/master/polliwog/transform/rigid_transform.py

• Stop using matrix functions from vg (opened lace/vg#95 about removing them)
• Add affine_transform
• Add apply_affine_transform (from vg.matrix.transform)

• Move coplanar_points_are_on_same_side_of_line to line_functions.
• Rename contains_coplanar_point to tri_contains_coplanar_point
• Rename as_rotation_matrix?
• Rename project_to_line to project_point_to_line
• Rename line_intersect3 to intersect_lines
• Rename line_intersect2 to intersect_2d_lines
• Rename partition to partition_segments and move after partition_segment

Hello everybody and thank @paulmelnikow and the rest of your staff for this lovely geometry package.

I am doing some tests to undestand well the basis and performances.
Though, I the method line.intersect gives always results even when the two line have not a intersection point. What's the meaning?
In the example below the lines below to two parallel planes.

# Importing
import numpy as np
from polliwog import Line

# 4 Points of a box on xy plane
x0y0=np.array([0.0, 0.0, 0.0])
x1y1=np.array([1.0, 1.0, 0.0])
x1y0=np.array([1.0, 0.0, 0.0])
x0y1=np.array([0.0, 1.0, 0.0])

line1 = Line.from_points(x0y0,x1y1) # line on xy plane
line2 = Line.from_points(x1y0,x0y1) # line on xy plane

print(f"Intersection {line1.intersect_line(line2)}") # right intersection point on the xy plane as expected

# now let's shift the plane of 1 along z
from polliwog import CompositeTransform
transform = CompositeTransform()
transform.translate(np.array([0.0, 0.0, 1.0]))
x0y0z1=transform(x0y0)
x1y1z1=transform(x1y1)

line3 = Line.from_points(x0y0z1,x1y1z1) # on xy plane but z++, no expected intersection
print(f"Intersection {line3.intersect_line(line2)}") # !! got array([-0.8660254, -0.8660254,  1.       ])

• Add tests (ref #52)
• Add documentation
• Use vg.major_axis, avoiding use of undeclared scipy dependency
• Use vg.shape.check

This follows the pattern of most of the other mutating functions.

Via lacecore:

>>> from lacecore import shapes
>>> m = shapes.cube(origin=np.zeros(3), size=1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/pnm/code/purl-anatomia/.venv/lib/python3.7/site-packages/lacecore/shapes/_shapes.py", line 50, in cube
    return _mesh_from_shape_fn(shapes.cube, origin=origin, size=size)
  File "/Users/pnm/code/purl-anatomia/.venv/lib/python3.7/site-packages/lacecore/shapes/_shapes.py", line 14, in _mesh_from_shape_fn
    *args, ret_unique_vertices_and_faces=True, **kwargs
  File "/Users/pnm/code/purl-anatomia/.venv/lib/python3.7/site-packages/polliwog/shapes/_shapes.py", line 102, in cube
    raise ValueError("`size` should be a number")
ValueError: `size` should be a number

e.g.

def point_nearest_to_line(
    reference_point_of_line,
    vector_along_line,
    query_points,
    ret_index=False,
):
    from polliwog.line import project_point_to_line

    vg.shape.check(locals(), "reference_point_of_line", (3,))
    vg.shape.check(locals(), "vector_along_line", (3,))
    vg.shape.check(locals(), "query_points", (-1, 3))

    projected = project_point_to_line(
        points=query_points,
        reference_points_of_lines=reference_point_of_line,
        vectors_along_lines=vector_along_line,
    )
    distances = vg.euclidean_distance(projected, query_points)
    closest_index = np.argmin(distances)
    closest = query_points[closest_index]
    return (closest, closest_index) if ret_index else closest

This project should have generated documentation on readthedocs, similar to https://entente.readthedocs.io/en/stable/ or https://vgpy.readthedocs.io/en/latest/.

It should also have doc builds in PRs, either through readthedocs or something like Netlify review apps.

e.g.

from polliwog import xy_plane

or

from polliwog import Plane
Plane.xy

I'm not completely sure the origin of the setter_property decorator. Could this be replaced by something in native Python, or perhaps replaced with something a little simpler?

Another alternative could be making Polyline fully immutable.