Add custom ExceptT (#37)

[turion]

This implementation of ExceptT has an Applicative instance
that does not need a Monad constraint of the underlying context.

[turion]

Do you want to have MaybeT treated in the same way? It would make sense, I guess.

[snowleopard]

Do you want to have MaybeT treated in the same way? It would make sense, I guess.

That sounds good but it may be best to finish the ExceptT story first.

Please ping me when this PR is good to review!

[turion]

Do you want to have MaybeT treated in the same way? It would make sense, I guess.

That sounds good but it may be best to finish the ExceptT story first.

Yes, agree. I wanted to upload this first stub so you can see the direction we're heading.

[snowleopard]

Do you want to have MaybeT treated in the same way? It would make sense, I guess.

That sounds good but it may be best to finish the ExceptT story first.

Yes, agree. I wanted to upload this first stub so you can see the direction we're heading.

My expectation is that MaybeT will be just like ExceptT with e = (), but sure feel free to share the first stub if you like!

[turion]

Do you want to have MaybeT treated in the same way? It would make sense, I guess.

That sounds good but it may be best to finish the ExceptT story first.

Yes, agree. I wanted to upload this first stub so you can see the direction we're heading.

My expectation is that MaybeT will be just like ExceptT with e = (), but sure feel free to share the first stub if you like!

Oh yes, I believe that as well. I rather meant that I wanted to share this PR here with this one file before I go on working everything out, since it contains the essential motivation for doing all that work.

[snowleopard]

Sure, sounds good!

[turion]

I don't know why I didn't mark it as ready for review. I believe it is.

[turion]

Ah, deriving MonadFail of course doesn't work for older GHCs. What's your policy for these situations? Deprecate older GHCs? Don't derive MonadFail? CPP?

[snowleopard]

Ah, deriving MonadFail of course doesn't work for older GHCs. What's your policy for these situations? Deprecate older GHCs? Don't derive MonadFail? CPP?

If it's just about MonadFail, then let's just write the instance by hand.

[snowleopard]

Could you test this transformer somehow? We do have a little testsuite for checking all Selective laws of various instances.

[turion]

Could you test this transformer somehow? We do have a little testsuite for checking all Selective laws of various instances.

Yes, I'd replicate the Writer test suite and adapt it.

But there is a problem: Where to put the Arbitrary instance for ExceptT? Putting it in the tests is an orphan instance. Also we would probably like to make the instance public so others can use it in their tests as well. But we don't want QuickCheck as a dependency to the main library.

I know two typical solutions to this situation:

• Orphan instance in tests
• Declare a cabal flag arbitrary-instance that conditionally activates the QuickCheck dependency. This flag is default false and can be set true for packages that want to test ExceptT (such as the test suite)

I like the second option more.

[turion]

Ah, we have to add an orphan instance for transformers' ExceptT anyways.

[turion]

I went for the first option for the time being. Are there any extra laws that make sense checking for ExceptT?

[turion]

It becomes more cumbersome to derive instances:

• MonadFail wasn't re-exported from Control.Monad in older GHCs, that's why older pipelines fail. If I explicitly import it, I get an unused-import warning on newer GHCs.
• Data.Functor.Contravariant doesn't exist in GHC < 8.6 (or rather the corresponding base).

Again the question what to do. Some options:

1. Drop older GHCs
2. CPP
3. Don't add instances for MonadFail and Contravariant

I personally like 1. by far the most. But of course the choice is yours.

[snowleopard]

@turion Apologies for the long silence! I somehow missed the notifications.

While I don't mind dropping compatibility with older GHCs in principle, doing this just to support MonadFail and Contravariant instances doesn't seem great. Let's just omit these instances for now. I doubt anyone will notice, to be honest :)

[snowleopard]

Are there any extra laws that make sense checking for ExceptT?

I'm not aware of any.

[snowleopard]

Orphan instance in tests

Yes, this is fine. Thanks!

[turion]

Ok, then I think I've done everything :)

[turion]

The build failure is caused because except used to have the type Either e a -> Except e a which changed to Either e a -> ExceptT e m a at some point. Older GHC versions somehow require older transformers versions indirectly. Again I'm not sure how to proceed. We can do CPP to have two definitions depending on GHC versions, drop Control.Selective.Trans.Except.except, or drop older GHCs.

[snowleopard]

The build failure is caused because except used to have the type Either e a -> Except e a which changed to Either e a -> ExceptT e m a at some point.

Ah, that's unfortunate. Let's use CPP to match the type for older GHC versions.

[turion]

If we already have CPP, shall I re-add MonadFail and Data.Functor.Contravariant?

[snowleopard]

If we already have CPP, shall I re-add MonadFail and Data.Functor.Contravariant?

Sure, let's do that. Thanks!

[snowleopard]

If we already have CPP, shall I re-add MonadFail and Data.Functor.Contravariant?

Sure, let's do that. Thanks!

Looks like it may be harder than it seemed. Feel free to skip this bit if you like!

[turion]

@snowleopard I could figure it out in the end. Ok like this? Then I'll squash.

[snowleopard]

Awesome, many thanks! I've squashed the commits.

I think it's a pretty cool new module :) I'll try to release a new version of the library soon.

[turion]

Awesome, this was great fun :)

[turion]

I just realised (again?) that the ExceptT instance is just the one from composing with a Traversable #12 (comment) I believe. So in the long run I think we should define another newtype plus instance for the general case and derive Selective that way.

[turion]

Also, was it really necessary to delete the old transformers instance? It's more restrictive than this one, but still it doesn't do any harm, right?

[snowleopard]

Agreed with both points! It would be cleaner to reuse a general Traversable based implementation in the ExceptT instance. And the old instance can remain, perhaps with a comment pointing to the existence of a less restrictive instance.