Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Orphan instance Wrapped (Point v) #29

Closed
bergey opened this issue Feb 2, 2014 · 6 comments
Closed

Orphan instance Wrapped (Point v) #29

bergey opened this issue Feb 2, 2014 · 6 comments

Comments

@bergey
Copy link
Member

bergey commented Feb 2, 2014

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 diagrams/diagrams-core@08257dd)
  • 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
Copy link
Member

byorgey commented Feb 4, 2014

What do we use the Wrapped instance for, anyway?

@jeffreyrosenbluth
Copy link
Member

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//issues/29#issuecomment-34055170
.

@bergey
Copy link
Member Author

bergey commented Feb 4, 2014

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
Copy link
Member

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//issues/29#issuecomment-34062319
.

@byorgey
Copy link
Member

byorgey commented Feb 4, 2014

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

bergey added a commit to diagrams/diagrams-core that referenced this issue Feb 4, 2014
bergey added a commit that referenced this issue Feb 4, 2014
@bergey
Copy link
Member Author

bergey commented Feb 4, 2014

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

@bergey bergey closed this as completed Feb 4, 2014
bergey added a commit to diagrams/vector-space-points that referenced this issue Mar 7, 2014
In favor of more geometrically correct approaches.
See: diagrams/diagrams-contrib#29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants