Add functionality for computing ego- and allocentric angles to RoIs

[willGraham01]

Description

What is this PR

• Bug fix
• Addition of a new feature
• Other

Why is this PR needed?

Introduces features necessary for computing ego- and allocentric boundary angles.

What does this PR do?

See the discussion in 370 for disambiguation of terms.

Several related methods are added to the RoI class. Most of these are self-evident from the name.

• compute_distance_to
• compute_nearest_point_to
• compute_approach_vector
• compute_allocentric_angle
• compute_egocentric_angle

These methods are added to the base region of interest class, so they can be used by all regions of interest.

In addition, LineOfInterest has also been given two additional methods;

• normal - returns the normal to the segment / line
• compute_angle_to_support_plane_of_segment - this performs the egocentric calculation, but using the normal vector $\vec{n}$ rather than the approach vector $\vec{a}$.

NOTE: There are a lot of features going in here which are interconnected, but the PR diff is still quite large. It is possible to break the addition of compute_distance_to and compute_nearest_point_to out into its own PR without too much difficultly, but all the other methods are very much interconnected and need to come in at the same time.

References

This functionality should close both #368 and #370.

In particular, we now have the functionality to do any of the calculations discussed in #370. This includes using the normal vector in place of the approach vector when considering a segment (compute_angle_to_support_plane_of_segment). Polygonal regions do not have this option, due to this behaviour being ill-defined at corners, but users can access the individual segments that make up the polygon's boundary, which are LineOfInterests, and then call compute_angle_to_support_plane_of_segment on the segment they deem appropriate.

How has this PR been tested?

Addition of tests to the test suite.
Dependence on shapely methods has been preferred throughout.

Is this a breaking change?

Does this PR require an update to the documentation?

Could potentially give more scope to the examples that are made as part of #415.

Checklist:

• The code has been tested locally
• Tests have been added to cover all new functionality
• The documentation has been updated to reflect any changes
• The code has been formatted with pre-commit

[sfmig]

great job consolidating all this work @willGraham01 !

The vast majority of my comments are about simplifying the function signatures, renaming variables to more intuitive names, and clarifying docstrings.

Re the implementation, I spotted an error with how the normal to a plane is computed. This is currently not captured by the tests because we test with a segment starting at the origin - I explain and provide examples in the comments. For the rest, it mostly looks good and clear to me. I mainly commented on things that may reduce the complexity (for example, removing _vector_from_centroid_of_keypoints or fixing some conventions to keep the function signatures short).

I have two additional more general comments:

• I think we should review these new ROI additions and make sure we highlight to the users when we assume the camera is top-down/bottom-up - this may be out of the scope of this PR though but something to keep in mind.
• We should think or check with the researchers (e.g. Sepi) what offset in angle with respect to a plane makes more sense in terms of interpretation. For example, if the head vector is at 0 deg wrt a wall or boundary - would that be interpreted as facing the wall? or is the animal's back to the wall? Not a big issue but maybe worth discussing. Right now, the animal is at 0deg when it has its back (of the head) to the wall.

I tagged @niksirbi in a few of the comments to get his view (and any other comments on the rest are more than welcome too). But I think to avoid duplicate effort it would be most useful if he reviews this PR after the suggested changes are reviewed. I also focused much less on the tests, so maybe that is something he can focus on more next.

[niksirbi]

Alright, I've gone through this now @willGraham01.

The main points that need attention are:

• some formatting errors in docstrings (I've pointed them out in individual comments)
• it's not clear to me what the support plane method will do in the case of a multi-line segment (>2 points). This needs clarification and testing.
• we should consider renaming the forward_vector to sth more general, I will tag you and Sofia in the relevant comment.

[willGraham01]

it's not clear to me what the support plane method will do in the case of a multi-line segment (>2 points). This needs clarification and testing.

Good catch as I had completely forgotten that we support cases like this! Right now it's always returning the normal to the first segment of a multi-line geometry, due to how the implementation is implemented.

Possible solutions:

• normal method takes an additional argument, which_segment for multi-segment geometries. This can either be an index or (probably more usefully) a point that the target segment should belong to. We can then delegate to the appropriate sub-geometry.
• Forbid use of normal on multi-segment geometries. Would be cleaner and less ambiguous, but restrictive of course.
• Return the normal for every segment in a multi-segment geometry (HARD: the broadcasting decorator won't be able to cope with this as it currently is).

Given the extra complexity this could potentially introduce, I'm inclined to go with the "only single-line segments" approach for now. Then we can extend on this in another PR if we want? Right now this PR is doing a lot already 😅

[sfmig]

Forbid use of normal on multi-segment geometries.

I think this makes sense for now.

Would a 4th option be: treat multi-segment geometries as polygons?

[willGraham01]

Would a 4th option be: treat multi-segment geometries as polygons?

normal isn't available to Polygons currently, but if we were to implement it in some way for Polygons then yeah we could do this too. Conversely we can use whatever we decide for multi-segment geometries for Polygons too.

[niksirbi]

Given the extra complexity this could potentially introduce, I'm inclined to go with the "only single-line segments" approach for now. Then we can extend on this in another PR if we want? Right now this PR is doing a lot already 😅

Because I agree re this PR being already too much, I'd say:

• enforce normals and angles to plane normals to only operate on single-segment lines.
• open an issue to think about future support of normals for muli-line segments and polygons. We may or may not end up doing this at some point, but I don't want to forget about it.

[willGraham01]

Have opened #439 so we don't forget about normals.

With that, I think all comments are addressed? So.... please review again guys? 😅

[niksirbi]

Approved by me, with some finishing touches.

[sfmig]

fantastic job @willGraham01 !

Some minor comments for consistency but otherwise looks nice and clear