BoundingBox is now Monoidal

[mgsloan]

No description provided.

[byorgey]

Also, I think the implementation of intersect is still incorrect -- it needs to have the check for outside' like the old version of intersect did. For example, consider when u = ((0,0), (1,1)) and v = ((2,2), (3,3)) -- currently intersect returns ((2,2), (1,1)), an invalid "nonempty" bounding box, when it should really return Empty.

[byorgey]

One other thing that occurs to me -- I went to start writing some simple QC tests for this module, but realized that I can't because of things not being exported. Can we refactor this into a module Diagrams.BoundingBox.Internal, which exports everything (but which is not itself exported from Diagrams.Prelude), and Diagrams.BoundingBox, which just re-exports the API we want to expose? That way a test module can import the Internal version.

(On second thought, perhaps this should wait until a separate patch.)

[mgsloan]

In order to resolve the "mempty" constraint issues, I've introduced "emptyBox".

Since the type of "intersections" is confusing and potentially error-prone (using the "Nothing" constructor to represent the box that includes everything), and "unions" is just "mconcat", I've removed them.

I think the "intersect" definition should work, as it uses "fromPoints" to check for that. This is more efficient than doing a separate "outside" check.

Why do the QC tests need to have access to the internals?

[mgsloan]

I suppose one reason to access internals is to check the results - but shouldn't Eq be enough? For some things, approximate equality might be useful, but even then, we can use "getCorners"

[byorgey]

Oh! Sorry, I see what you mean now re: intersect -- yes, I think the call to fromCorners handles everything properly.

Re: access to internals for writing QC tests, the issue is that we need to be able to write an Arbitrary instance for NonEmptyBoundingBox. I suppose one could argue that we can just write an Arbitrary instance for BoundingBox which uses fromCorners -- but I'd like to be able to write tests for fromCorners itself!

[mgsloan]

Thanks! Yeah, I'm glad about how the changes turned out overall. This
makes diagrams incrementally more similar to lib2geom, w.r.t. bounding
boxes. We may want to go even further, and have them be equivalent to
points where the vectorspace scalars are intervals (one of my proudest
lib2geom innovations ;) )

Imho, fromCorners is trivially correct if its used to implement the other
tests. We should test that it yields empty for invalid corners, though.
On Aug 1, 2012 1:57 PM, "Brent Yorgey" <
reply@reply.github.com>
wrote:

Oh! Sorry, I see what you mean now re: intersect -- yes, I think the
call to fromCorners handles everything properly.

Re: access to internals for writing QC tests, the issue is that we need to
be able to write an Arbitrary instance for NonEmptyBoundingBox. I
suppose one could argue that we can just write an Arbitrary instance for
BoundingBox which uses fromCorners -- but I'd like to be able to write
tests for fromCorners itself!

---

Reply to this email directly or view it on GitHub:
#2 (comment)