Orphan instance Wrapped (Point v)

diagrams / diagrams-contrib

User-contributed extensions to diagrams

BSD 3-Clause "New" or "Revised" License

27 stars 30 forks source link

Orphan instance Wrapped (Point v) #29

Closed bergey closed 10 years ago

bergey commented 10 years ago

There's a Wrapped instance for Points that clearly doesn't belong in diagrams-contrib, but I'm not sure where it belongs. I've considered:

It belongs in vector-space-points, where it's not an orphan, but adds a lens dependency.
It belongs in Diagrams.Core.Points (as I do in https://github.com/diagrams/diagrams-core/commit/08257dd2ab05f88fc0358e070ffa4a858cf00be2)
There shouldn't be a Wrapped instance, because we want people to think harder about the semantics of .-. origin. There should be an Iso with some more descriptive name.

Preferences?

byorgey commented 10 years ago

What do we use the Wrapped instance for, anyway?

jeffreyrosenbluth commented 10 years ago

I think I moved it from michael sloan's: https://github.com/diagrams/diagrams-contrib/blob/master/src/Diagrams/Lens.hs

On Tue, Feb 4, 2014 at 7:38 AM, Brent Yorgey notifications@github.comwrote:

What do we use the Wrapped instance for, anyway?

Reply to this email directly or view it on GitHubhttps://github.com/diagrams/diagrams-contrib/issues/29#issuecomment-34055170 .

bergey commented 10 years ago

I don't think we're currently using this Wrapped instance at all. vector-space-points exports a Newtype instance, and I assumed we'd want a Wrapped instance because we replaced all the other Newtype instances this way. diagrams-core and diagrams-lib don't use the Newtype instance, either.

jeffreyrosenbluth commented 10 years ago

Yes I agree, let's get rid of it.

On Tue, Feb 4, 2014 at 9:12 AM, Daniel Bergey notifications@github.comwrote:

I don't think we're currently using this Wrapped instance at all. vector-space-points exports a Newtype instance, and I assumed we'd want a Wrapped instance because we replaced all the other Newtype instances this way. diagrams-core and diagrams-lib don't use the Newtype instance, either.

Reply to this email directly or view it on GitHubhttps://github.com/diagrams/diagrams-contrib/issues/29#issuecomment-34062319 .

byorgey commented 10 years ago

OK, let's get rid of it, and get rid of the Newtype instance too while we're at it.

bergey commented 10 years ago

I removed the Newtype instance, the copy in my -core PR, and the -contrib copy.