Expand classes to avoid needing some rules

[treeowl]

Some RULES pragmas are used to specialize functions to specific graph implementations. I believe most or all of these could be eliminated by adding the relevant functions to Graph or DynGraph. If you are concerned that some of them may not really belong in the API, and don't know what user-friendly methods to add to support them, you can hide those methods by defining the class in an "internal" module, and only export some of its methods. Using the class system instead of rules, when possible, guarantees that the preferred implementation will always be used.

[ivan-m]

Yeah, I'm not a big fan of using RULES for this, but I'm hesitant to move these functions to the classes whilst for the most part development on fgl is progressing in an improvement fashion (adding docs, adding useful functions, etc.) that are mainly backwards compatible; changing the typeclass definitions is more of a major version change.

[treeowl]

It's possible to make the change in a way that's invisible to users,
although it can be made visible later. See, for example,
Data.Profunctor.Unsafe, which defines the Profunctor class including some
methods not exposed through Data.Profunctor. The default definitions ensure
that even users importing only Data.Profunctor can write valid (if perhaps
somewhat inefficient) instances. I believe the public module can just
import the methods as functions.

import Secret (Graph(.....), gmap, ....)

There are some other ideas I have that would be more major-versiony than
this. I'd love to get more specific about some types, turn some tuples into
records, make some things stricter, and make it easier to ensure that code
doesn't accidentally rely on Node=Int.

On Aug 29, 2016 9:52 PM, "Ivan Lazar Miljenovic" notifications@github.com
wrote:

Yeah, I'm not a big fan of using RULES for this, but I'm hesitant to move
these functions to the classes whilst for the most part development on fgl
is progressing in an improvement fashion (adding docs, adding useful
functions, etc.) that are mainly backwards compatible; changing the
typeclass definitions is more of a major version change.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#38 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABzi_a43M6FM-FJMQ7_2ogBpSFs12W28ks5qk4zxgaJpZM4JwDbm
.

[treeowl]

Oh yes, and quit that horrible "Check for empty, then use a partial
function to decompose" pattern.

On Aug 29, 2016 10:05 PM, "David Feuer" david.feuer@gmail.com wrote:

It's possible to make the change in a way that's invisible to users,
although it can be made visible later. See, for example,
Data.Profunctor.Unsafe, which defines the Profunctor class including some
methods not exposed through Data.Profunctor. The default definitions ensure
that even users importing only Data.Profunctor can write valid (if perhaps
somewhat inefficient) instances. I believe the public module can just
import the methods as functions.

import Secret (Graph(.....), gmap, ....)

There are some other ideas I have that would be more major-versiony than
this. I'd love to get more specific about some types, turn some tuples into
records, make some things stricter, and make it easier to ensure that code
doesn't accidentally rely on Node=Int.

On Aug 29, 2016 9:52 PM, "Ivan Lazar Miljenovic" notifications@github.com
wrote:

Yeah, I'm not a big fan of using RULES for this, but I'm hesitant to
move these functions to the classes whilst for the most part development on
fgl is progressing in an improvement fashion (adding docs, adding useful
functions, etc.) that are mainly backwards compatible; changing the
typeclass definitions is more of a major version change.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#38 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABzi_a43M6FM-FJMQ7_2ogBpSFs12W28ks5qk4zxgaJpZM4JwDbm
.

[ivan-m]

Well, the latter will definitely require a major version bump ;-)

In reality, I'd like to have the time to write a new graph library from the ground up... but I've been saying that for about 10 years now >_>

What makes expanding the class major version bump required is from how you have to import functions; this is classified as a breaking change by the Package Versioning Policy.

[treeowl]

It's definitely possible to avoid that. At worst, you can use

module Public where
import qualified Secret
gmap :: ...
gmap = Secret.gmap

but I don't think you have to go that far. I believe that having the
public module import or export the methods outside the parentheses will
turn the methods into functions to the outside.

On Aug 29, 2016 10:11 PM, "Ivan Lazar Miljenovic" notifications@github.com
wrote:

Well, the latter will definitely require a major version bump ;-)

In reality, I'd like to have the time to write a new graph library from
the ground up... but I've been saying that for about 10 years now >_>

What makes expanding the class major version bump required is from how you
have to import functions; this is classified as a breaking change
http://pvp.haskell.org/ by the Package Versioning Policy.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#38 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABzi_cPTg6pQHT3wvtS1YH-B3Ir6MvW3ks5qk5FqgaJpZM4JwDbm
.

[treeowl]

Ugh. I just checked. The easy ways don't work, as you expected (silly
design decision that). But the ugly sledgehammer of writing functions
implemented using secret methods works just fine.

On Mon, Aug 29, 2016 at 10:19 PM, David Feuer david.feuer@gmail.com wrote:

It's definitely possible to avoid that. At worst, you can use

module Public where
import qualified Secret
gmap :: ...
gmap = Secret.gmap

but I don't think you have to go that far. I believe that having the
public module import or export the methods outside the parentheses will
turn the methods into functions to the outside.

On Aug 29, 2016 10:11 PM, "Ivan Lazar Miljenovic" <
notifications@github.com> wrote:

Well, the latter will definitely require a major version bump ;-)

In reality, I'd like to have the time to write a new graph library from
the ground up... but I've been saying that for about 10 years now >_>

What makes expanding the class major version bump required is from how
you have to import functions; this is classified as a breaking change
http://pvp.haskell.org/ by the Package Versioning Policy.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#38 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABzi_cPTg6pQHT3wvtS1YH-B3Ir6MvW3ks5qk5FqgaJpZM4JwDbm
.

[ivan-m]

Actually, I think the best solution for this would be Backpack if/when it becomes usable. But back in the real world...

I suppose my main concern about this kind of hidden class change is that on the off chance someone else is implementing their own instances of Graph and DynGraph then this will result in having to update their code twice: once to use the new hidden version, then again when it's made public.

[treeowl]

They absolutely do not need to update their code the first time. Their code
will continue to do exactly what it was doing before; the method will
simply take its default definition, which will be the same as the
definition of what is now a function.

On Mon, Aug 29, 2016 at 11:57 PM, Ivan Lazar Miljenovic <
notifications@github.com> wrote:

Actually, I think the best solution for this would be Backpack
https://ghc.haskell.org/trac/ghc/wiki/Backpack if/when it becomes
usable. But back in the real world...

I suppose my main concern about this kind of hidden class change is that
on the off chance someone else is implementing their own instances of
Graph and DynGraph then this will result in having to update their code
twice: once to use the new hidden version, then again when it's made
public.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#38 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABzi_Vq6VYHg6rHaQ-RBqf38X98-znd8ks5qk6pFgaJpZM4JwDbm
.

[ivan-m]

To clarify: if they want to take advantage of the new classes to not have to use their own RULEs then they'd need to update twice.

And it still breaks the PVP, so I'm not sure whether it could potentially have other impacts.

I have no problem with the next version being a major version bump with this in there, though planning around exactly where the border between being in the class and being a derived function will need to be determined.

[ivan-m]

(I suppose if nothing else updating FGL to have a larger type class gives me something to do at ICFP :p)