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

Allows us to write Diagram B instead of Diagram B V2 Double/Float/Whatever in diagrams programs #62

Merged
merged 2 commits into from
Oct 17, 2014

Conversation

jeffreyrosenbluth
Copy link
Member

Currently the linear changes has made the B token less useful since the type Diagram B V2 n the n depends on B the backend.
This provides the ability to switch backends simply by changing the import by using Diagram B instead of Diagram B V2 Double e.g.

  • Companion branch in core

@jeffreyrosenbluth
Copy link
Member Author

I can add this to the other backends if everyone likes it.
I'm also open to the names other than Figure.

@cchalmers
Copy link
Member

I like the idea. How about abusing the existing V and N type families instead of making a new one?

type instance V SVG = V2
type instance N SVG = Double

type Figure b = Diagram b (V b) (N b)

-- possible (bad) examples
defaultRender :: Mainable (Figure b) => FilePath -> SizeSpec (V n) (N b) -> Figure b -> IO ()
forBackend :: SameSpace a b -> b -> a -> a

I'd also like to nominate the names Picture and Dia.

@jeffreyrosenbluth
Copy link
Member Author

Yes that makes sense. I'll make the change and see check that it works.
I think we should probably go with Dia as a name as Picture is likely to conflict
with lots of other libraries.

@jeffreyrosenbluth
Copy link
Member Author

@cchalmers Unfortunately, this does not seem to work.
The type checker can't seem to work it out and in diagrams
programs gives errors like "Expected type Dia B, Actual type Dia B"
pretty funny error though

@jeffreyrosenbluth
Copy link
Member Author

I'm starting to like the pun Monogram fro monomorphic diagram instead
of Dia or Figure. Thoughts?

@cchalmers
Copy link
Member

That's weird, it works when I try it.

I'm not sure about Monogram I like the pun but I don't really think about a diagram when I see it.

@jeffreyrosenbluth
Copy link
Member Author

What do you mean works? It compiles fine but when I try it in a diagrams programs with serveral
Dia B type annotations it gets confused.

@jeffreyrosenbluth
Copy link
Member Author

Here is a simple example

{-# LANGUAGE NoMonomorphismRestriction #-}

module Main where

import Diagrams.Prelude
import Diagrams.Backend.SVG.CmdLine

main = mainWith $ example # frame 1

c :: Dia B
c = circle 1 

s :: Dia B
s = square 2

example :: Dia B
example = c <> s

with error:
[1 of 1] Compiling Main ( Dia.hs, Dia.o )

Dia.hs:17:11:
Couldn't match type ‘V B’ with ‘V2’
Expected type: Dia B
Actual type: Dia B
In the first argument of ‘(<>)’, namely ‘c’
In the expression: c <> s

@cchalmers
Copy link
Member

That example works. Couldn't match type ‘V B’ with ‘V2’ makes me think you didn't write

type instance V SVG = V2

If you did then that's very strange.

@jeffreyrosenbluth
Copy link
Member Author

Yup I forgot that and type instance N SVG = Double
I'm still not loving Dia how about Illustration.

@fryguybob
Copy link
Member

I'm not a fan of taking up more of the name space for a convenience. While it isn't the greatest I think it is less confusing to be something with Diagram in the name. Perhaps BDiagram for backend fixed diagram? Also do we gain anything by having the B argument? I seem to remember there were reasons for not going with type B = Diagram SVG R2 back when we introduced B, but I can't think of them now.

@jeffreyrosenbluth
Copy link
Member Author

That's a good point about the name.
Maybe we should lose Diagram since it's barely better thatn QDiagram,
and swich Dia to Diagram.

I dont remember why we didn't use type B = ... either?

@byorgey
Copy link
Member

byorgey commented Oct 16, 2014

Maybe we should lose Diagram since it's barely better thatn QDiagram,
and swich Dia to Diagram.

Yes, Diagram was originally intended to be essentially what you are discussing here (a convenient synonym that is sufficient for everyday use but hides some type complexity). So this sounds like a good route to me.

@jeffreyrosenbluth
Copy link
Member Author

@byorgey nice to see you chiming in :)

@byorgey
Copy link
Member

byorgey commented Oct 16, 2014

Well, you know, I'm all PhD'd and stuff now. =) I still have tons on my plate but I hope to be phasing back in a bit of time spent on diagrams.

@jeffreyrosenbluth
Copy link
Member Author

Well I guess congratulations are in order Dr. Byorgey :D.
We all look forward to having you back.

@byorgey
Copy link
Member

byorgey commented Oct 16, 2014

=D Thanks!

@jeffreyrosenbluth jeffreyrosenbluth changed the title Allows us to write Figure B instead of Diagram B V2 Double/Float/Whatever in diagrams programs Allows us to write Diagram B instead of Diagram B V2 Double/Float/Whatever in diagrams programs Oct 16, 2014
@jeffreyrosenbluth
Copy link
Member Author

I think this, core and svg are ready to go.

@jeffreyrosenbluth
Copy link
Member Author

I'd like to go ahead and merge all of these token branches. It's not a very controversial change and gets us back to why we created the Diagram type alias in the first place. Since the changes effect so many repos it will become a burden to sync them with main if some other branch is merged first.

As far as other aliases are concerned, as a library writer I'm happy to use the more verbose type e.g. QDiagram Cairo V2 Double Any. One can always define a convenient alias in a particular module or application program. In other words I don't think we should create too many aliases in -core.

@bergey
Copy link
Member

bergey commented Oct 17, 2014

I like the design currently in this branch, with Diagram B filling in the v and n for the Backend. +1 to merge.

jeffreyrosenbluth added a commit that referenced this pull request Oct 17, 2014
Allows us to write `Diagram B` instead of `Diagram B V2 Double/Float/Whatever` in diagrams programs
@jeffreyrosenbluth jeffreyrosenbluth merged commit 9976545 into master Oct 17, 2014
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

Successfully merging this pull request may close these issues.

5 participants