-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add Foldable
, Traverse
and Comonad
instances to WriterT
#2182
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
0f3b16c
Added foldLeft and foldRight to WriterT and Foldable instance
barambani 4d1d7f7
Added tests for Foldable instance
barambani 241f213
Added traverse. Fixed priority of the implicit search for isntances
barambani e68849d
Added traverse implementation to WriterT
barambani 6729608
Added test for WriterT Traverse
barambani 2a7f5f2
Refactored names
barambani 454cc8f
Changed priority of Traverse instance
barambani 4cf8304
Added comonad instance for WrtiterT and Const. Added tests
barambani 0c2c002
Completed Comonad tests using tuple instead of Const
barambani e63a7e3
Refactored extract derivation to avoid implicit search
barambani fba0f7a
Added instances and reolution tests for Id. Removed unneeded type
barambani 2e4b2b5
Minor refactoring
barambani fd077c3
Attempt to increase coverage
barambani a40e9f7
Removed unneeded serialization test
barambani b3ab664
Refactored traverse derivation
barambani 152ee7a
Fixed improper inheritance of WriterTFunctor
barambani File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@barambani you can't have any instance that potentially conflicts in the same trait level.
I think there is
Functor
instance conflict here for[WriterT[Id, L, ?]
. I think you might need to create quite a few more priority layers to separate the instances.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be clear I didn't see the actually error message on travis. But I think separating them out might solve the problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the confusion, my bad for the lack of clarity. This prioritisation should work (let's see when the CI completes). My question was more in general about the reason why we needed to create instances for
Id
. I would have expected that something likeComonad[WriterT[Id, ListWrapper[Int], ?]]
could summon the instance forF[_]
and I was wondering if you had resources about why that's not the case.This doesn't mean that I'm not happy to add more priority levels if you think that's better. Thanks for the great help here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@barambani I don't know the underlying reason that
Id
needs to be special-cased, but it's a weakness in scalac's type inference. This is a related Stack Overflow post that I've seen before. Something likeId
might be a tricky one becauseA =:= Id[A] =:= Id[Id[A]]
, so there may be some logic to short-circuit inference when applying a type results in the same type so you don't end up in an infinite loop or something. This is pure speculation :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ceedubs thanks a lot, I didn't want for you to waste time on this. I was just taking advantage of this discussion in case you knew it out of your head 😃 👍