Open treeowl opened 8 years ago
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.
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 https://github.com/haskell/fgl/issues/38#issuecomment-243310402, or mute the thread https://github.com/notifications/unsubscribe-auth/ABzi_a43M6FM-FJMQ7_2ogBpSFs12W28ks5qk4zxgaJpZM4JwDbm .
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 https://github.com/haskell/fgl/issues/38#issuecomment-243310402, or mute the thread https://github.com/notifications/unsubscribe-auth/ABzi_a43M6FM-FJMQ7_2ogBpSFs12W28ks5qk4zxgaJpZM4JwDbm .
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.
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 https://github.com/haskell/fgl/issues/38#issuecomment-243313107, or mute the thread https://github.com/notifications/unsubscribe-auth/ABzi_cPTg6pQHT3wvtS1YH-B3Ir6MvW3ks5qk5FqgaJpZM4JwDbm .
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 https://github.com/haskell/fgl/issues/38#issuecomment-243313107, or mute the thread https://github.com/notifications/unsubscribe-auth/ABzi_cPTg6pQHT3wvtS1YH-B3Ir6MvW3ks5qk5FqgaJpZM4JwDbm .
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.
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 https://github.com/haskell/fgl/issues/38#issuecomment-243326456, or mute the thread https://github.com/notifications/unsubscribe-auth/ABzi_Vq6VYHg6rHaQ-RBqf38X98-znd8ks5qk6pFgaJpZM4JwDbm .
To clarify: if they want to take advantage of the new classes to not have to use their own RULE
s 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.
(I suppose if nothing else updating FGL to have a larger type class gives me something to do at ICFP :p)
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 toGraph
orDynGraph
. 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.