[generics-rep] Add instances for some usual algebraic types?

[kindaro]

As I understand from documentation, instances for Generic may only be defined in either:

• The module that defines the subject type.
• The module that defines the class Generic.
• The module that defines the type used to construct the generic representation.

In the case of Generic, the latter two coincide in Data.Generic.Rep.

This means that I cannot possibly define an instance of Generic for any type I do not define myself in the first place. This includes all the usual types like Unit or Either. The only instance provided by this package is for Maybe — this is not enough. I propose that more instances be given.

Since generics are such a basic thing, I further propose that the right place to define instances is in the modules that define subject types. This includes the Maybe instance — it should be moved away for consistency.

[garyb]

The dependency for Maybe can't be reversed as this library uses it in the implementation of GenericEnum.

[garyb]

Although perhaps GenericEnum could be provided in purescript-enums instead too, it might well work out then.

[kindaro]

@garyb  Am I to understand that you are in favour of this undertaking in principle?

[paulyoung]

#34 (comment) says:

The instance for Either should live in purescript-either instead, and should be derivable.

So I did that in purescript/purescript-either#57

[garyb]

@kindaro yeah, it seems reasonable to me - since Generic has compiler support it's pretty special in the ecosystem anyway, so moving it slightly closer to the root of the dependency hierarchy isn't a problem in my book.

Deriving the instance in -maybe rather than having to hand write it the way it is just now will be nice too.

I think if we were to move the GenericEnum stuff it should probably go as Data.Enum.Generic. That pattern is the usual way it works for situations where generic helpers are provided outside of this library. Actually, there's an argument that this pattern should be used in all cases for consistency. This library could then have no dependencies, and either be merged into Prelude, or -prelude could become a dependent and the classes moved there. That's a bit more of a controversial stance perhaps though, would be interesting to see if other core team members have thoughts. @hdgarrood might be in favour, since he's expressed interest in having a larger single Prelude already.

[hdgarrood]

I like the idea of using the same pattern in all cases, and I also think moving Generic closer to the root of the dependency hierarchy makes sense, for the same reason. I would be in favour of merging this library into prelude, assuming we don't plan to re-export any of it from the Prelude module.

[garyb]

Yeah, that was what I was thinking - just changing where it resides, not the Prelude module.

[kl0tl]

Is this something we should do in 0.14?

[JordanMartinez]

I think so.

[hdgarrood]

Unless this is likely to be breaking in some way (I don’t think it is?), then I think we should consider postponing it so that we can get the release out sooner.

[JordanMartinez]

Unless this is likely to be breaking in some way (I don’t think it is?)

So, here's my understanding of what we'd have to change:

• move this repo (excluding the GenericEnum module) into the purescript-prelude repo. However, we do not re-export its contents from the Prelude module. The modules names stay the same.
• move this repo's GenericEnum module, which depends on maybe into purescript-enums. purescript-enums already depends on maybe, so this isn't a breaking change there.
• Add instances to other repos (e.g. Unit in prelude; either in Either; Maybe in maybe, etc.)

The only breaking changes we might make are the names of the modules. Quoting Gary:

I think if we were to move the GenericEnum stuff it should probably go as Data.Enum.Generic. That pattern is the usual way it works for situations where generic helpers are provided outside of this library. Actually, there's an argument that this pattern should be used in all cases for consistency.

and Harry:

I like the idea of using the same pattern in all cases

Renaming the modules would be a breaking change. If we do not rename them, we could migrate the modules after v0.14.0. If we rename them, we should do it before v0.14.0. Which one should we take?

I believe the modules would be renamed to the following:

• Data.Generic.Rep.Bounded -> Data.Bounded.Generic
• Data.Generic.Rep.Enum -> Data.Enum.Generic
• Data.Generic.Rep.Eq -> Data.Eq.Generic
• Data.Generic.Rep.HeytingAlgebra -> - Data.HeytingAlgebra.Generic
• Data.Generic.Rep.Monoid -> Data.Monoid.Generic
• Data.Generic.Rep.Ord -> Data.Ord.Generic
• Data.Generic.Rep.Ring -> Data.Ring.Generic
• Data.Generic.Rep.Semigroup -> Data.Semigroup.Generic
• Data.Generic.Rep.Semiring -> Data.Semiring.Generic
• Data.Generic.Rep.Show -> Data.Show.Generic

[hdgarrood]

Yes, good point, thanks. I’m not sure why I thought this wouldn’t be breaking. I agree then, now probably is a good time to do it.

[hdgarrood]

Unit shouldn’t get a Generic instance, incidentally. We should only provide Generic instances when a data type’s constructors are exported. But other than that this all sounds good 👍

I think doing it now and renaming the modules sounds preferable. Moving a module from one library to another is always sort of breaking anyway, because you’ll end up with duplicate module errors if you don’t drop the library where those modules previously lived from your dependencies.

[JordanMartinez]

See purescript/purescript-prelude#235

[JordanMartinez]

Things left to do:

• Port most code to purescript-prelude (Port generics-rep to prelude purescript/purescript-prelude#235)
• Port the Enum code to purescript-enums (Port Enum parts of generics-rep to this repo purescript/purescript-enums#46)
• Add Maybe's Generic instance to purescript-maybe (Port Maybe's Generic instance to this repo purescript/purescript-maybe#55)
• Add Either's Generic instance to purescript-either (Complete PR adding Generic instance for Either purescript/purescript-either#61)
• Add Tuple's Generic instance to purescript-tuples (feat: Derive Generic instance (related to https://github.com/purescript/purescript-generics-rep/pull/34) purescript/purescript-tuples#40)
• Transfer all issues in this repo to prelude or enums repos
• Deprecate this library

Anything else?

[JordanMartinez]

All other PRs pass CI and are ready for review. See above comment for links to them.

[JordanMartinez]

All PRs have been merged. The only thing left to do here is transfer the issues and deprecate this repo.

[kindaro]

Woo-hoo, such progress in such a short time. Thank you Jordan and allies.

[JordanMartinez]

I renamed all issues in this repo, so that they include "[generics-rep] " in front of the original issue. I've transferred all open issues in this repo to purescript-prelude. Not sure whether I should also transfer this one.

There's two pull requests currently open. I'm guessing we should close them and open issues in prelude for them if they are still desired.

[JordanMartinez]

Ok. All PRs have been addressed:

• I reimplemented Add Show instances for the Rep types #11 in Complete daniel-chambers' PR adding Show instances to generics rep types purescript/purescript-prelude#250 and will get that fixed later this week.
• We decided Add and derive Deep class #7 wasn't worth doing at this time.

So, I believe the only thing left to do is deprecate this repo.

[JordanMartinez]

As stated above, I believe the only thing left to do is to deprecate this repo. Does someone need to transfer this repo to the purescript-deprecated organization and update its Readme to clarify why it was deprecated?

[thomashoneyman]

I haven’t been following closely enough to judge if everything else is ready to go, but I can take care of deprecating this library if so.

[thomashoneyman]

I've moved this library to the deprecated organization. So long as no one remembers something else we need to do (@hdgarrood, @kl0tl?), I can complete the deprecation process tomorrow night and upload the notice to Pursuit.

[hdgarrood]

I think it might be best to wait until after 0.14.0 is released to tell Pursuit that this library has been deprecated, because we want the current version to still appear in search results until then.

[thomashoneyman]

That's a good reason to wait; I'll leave this issue open.