Making largestDimension work as expected

[jmwright]

WIP - do not merge.

Intended to address #266 but I've run into a couple of issues.

1. Several tests fail now because largestDimension is used for automatically determining hole depth. I don't think the original method was doing this correctly, and I need to get this sorted out before this PR is viable.
   Ex:

   cadquery/cadquery/cq.py

   Line 2347 in 6ecb0fc

   depth = self.largestDimension()

I think that's a reason that only the bounding box of the first solid was checked. You wouldn't want the hole depth to be based on all the solids in a compound (or would you?). In any case, I suspect the original implementation was kind of a hack to get hole depth to work properly. I need to think about this a bit to figure out how to refactor the code for hole depth.

2. I'm not getting the enclosing sphere diameter that I expected. For a unit cube (1x1x1), I would expect the diagonal (which is the diameter of the enclosing sphere?) to be sqrt(1^2 + 1^2) or 1.414, but my method using the Vector length yields about 1.76. I'm probably missing something simple in vector length vs diagonal/hypotenuse using two sides of the square.

Thoughts and guidance welcome.

@adam-urbanczyk @greyltc

[jmwright]

Nevermind on issue 2. My brain was stuck thinking in terms of 2d vectors, not 3d. 1.76 is correct for a 3d diagonal of a 3d unit cube.

[jmwright]

Reworked the code a little bit which now gives a slightly larger answer. BoundingBox().DiagonalLength, which was used before, consistently gives a value on the low side.

It seems that this method was just created to serve thru cuts by returning a distance that would be sure to be through the object. I think that's where the multiplication by 5 came in. It was was a hard coded overkill value to make sure it always returned a value that was long enough. That's fine when all it does is thru cuts. However, when you try to use this function to aid in setting something like camera distance, it gives the wrong answer.

It now gives a correct (and intuitive) answer, while not breaking the tests for thru cuts.

[codecov]

Codecov Report

Merging #317 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #317   +/-   ##
=======================================
  Coverage   95.38%   95.39%           
=======================================
  Files          19       19           
  Lines        4701     4709    +8     
=======================================
+ Hits         4484     4492    +8     
  Misses        217      217           

Impacted Files	Coverage Δ
cadquery/occ_impl/shapes.py	90.81% <ø> (+0.03%)	⬆️
cadquery/cq.py	94.01% <100.00%> (+0.01%)	⬆️
tests/test_cadquery.py	99.21% <100.00%> (+<0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ecb0fc...71196ba. Read the comment docs.

[jmwright]

@adam-urbanczyk It's a small-ish change, but could potentially impact thru cuts. Probably worth a quick check before I merge, even though I know you're busy with OCP.

[adam-urbanczyk]

Thanks @jmwright !