Create Plane from Location

[Joschua-Conrad]

First of all: I love working with CQ, thank you for maintaining this library!

Plane supports conversion between local/global coordinate systems, and Location supports chaining coordinate-system offsets.
So I from time to time want to convert between them.
However, while there is a Location.__init__ from Plane and a Plane.location property, I did not find an implementation of the opposite conversion.
This PR adds this conversion from Location to Plane.

Some things I ask the reviewers to notice and possibly discuss:

• I tried using @multimethod on Plane.__init__. But that then disallows passing keyword arguments to the constructor, and that is needed in existing code. Using @multidispatch produces working code, but Sphinx docs do not support it. So I left Plane.__init__ untouched and added a Plane.fromLocation class-method.
• The Location.plane property I added is analogous to Plane.location.
• I did not find a test of geom.py, so I added test_geom.py. The new test creates a Plane and then runs through a chain of conversions Plane -> Location -> Plane -> Location. In the end, there are two Planes and two Locations, which have to be equal, respectively. This is asserted, along with origin and rotations of the first created Location.
• pytest, build_docs.sh, and black all pass, and yield the same warnings as on the master branch.

I'm excited about your feedback!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.75%. Comparing base (dadf151) to head (e5b9c30).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1912   +/-   ##
=======================================
  Coverage   95.74%   95.75%           
=======================================
  Files          29       29           
  Lines        7827     7845   +18     
  Branches     1179     1179           
=======================================
+ Hits         7494     7512   +18     
  Misses        192      192           
  Partials      141      141           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jmwright]

Thanks @Joschua-Conrad , I like the thorough commenting. I asked a question and made a couple inline suggestions, but otherwise it looks good.

[Joschua-Conrad]

Thanks @Joschua-Conrad , I like the thorough commenting. I asked a question and made a couple inline suggestions, but otherwise it looks good.

Thank you for the review and for fixing my spelling mistakes!

[Joschua-Conrad]

As @lorenzncode brought up speed issues with multidispatch vs. multimethod, I built a minimum example using timeit to investigate that.

The example profiles a simple overloaded function with multidispatch, the same function with multimethod, and the Plane.__init__ constructor. I retrieved the results for Plane for a multidispatch implementation (as-is), for a multimethod implementation (maybe faster, but does not support keyword arguments), and for a non-overloaded implementation (using @classmethod, just revert cd99615). All the different functions and overloads are also tested with different argument types, to see whether the chosen @multidispatch version has an influence.

These are my results. avg, min, and max are gathered over 10 repetitions:

Function	Number	Arguments	Min [s]	Max [s]	Avg [s]	Typing
Plane.__init__	10k	Vector	0.657	0.663	0.661	classmethod
Plane.__init__	10k	Vector	1.197	1.267	1.224	multimethod
Plane.__init__	10k	Vector	0.673	0.679	0.676	multidispatch
Plane.__init__	10k	Location	1.411	1.482	1.425	classmethod
Plane.__init__	10k	Location	1.193	1.219	1.204	multimethod
Plane.__init__	10k	Location	1.165	1.177	1.169	multidispatch
Custom	1M	int	1.277	1.345	1.294	multimethod
Custom	1M	int	1.173	1.239	1.196	multidispatch
Custom	1M	str	1.418	1.441	1.426	multimethod
Custom	1M	str	1.294	1.304	1.299	multidispatch

Always two or three lines above each other can be compared. The Plane.__init__ from Vector with classmethod is the fastest implementation for my PR (Avg 0.661s), as it does not utilize any overloading. @multidispatch adds only a slight overhead, while @multimethod adds a huge overhead (Avg 1.224s). Between these two, I only changed @multidispatch to @multimethod. These results are not as expected, as @multidispatch is faster.

Plane.__init__ from Location in general takes longer due to more complex initialization, such that the influence of @multidispatch to @multimethod is less expressed.

For a small custom function, the difference between @multidispatch and @multimethod can almost not be seen, but @multidispatch is again slightly faster.

TL;DR In my example, @multidispatch (and hence the implementation as it is right now) is slightly faster. This is not how we expect it. But the difference is negligible when compared to the time consumed by a non-trivial constructor.

[lorenzncode]

Thanks @Joschua-Conrad !

In the PR description it says there is (currently before PR) Location.plane. I think it should be Plane.location.

However, while there is a Location.init from Plane and a Location.plane property, I did not find an implementation of the opposite conversion.

[Joschua-Conrad]

In the PR description it says there is (currently before PR) Location.plane. I think it should be Plane.location.

You're right, I fixed that in the PR description. Thanks for noticing, and thank you for your review!

[adam-urbanczyk]

Alright, merging. Thanks for the PR @Joschua-Conrad !