-
Notifications
You must be signed in to change notification settings - Fork 64
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
SetContainer is not derivable #195
Comments
I'd rather not have a breaking API change, but I'm open to a PR that keeps backwards compatibility. |
Don't see how you could have both. As I said, the key problem is that the signature of unions makes SetContainer impossible to derive. Fixing it either requires moving unions out or changing its signature.
On July 6, 2021 4:44:04 AM GMT+03:00, Michael Snoyman ***@***.***> wrote:
I'd rather not have a breaking API change, but I'm open to a PR that keeps backwards compatibility.
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#195 (comment)
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
Then I don’t think we can make the change |
Fair enough. Will look for another way of doing what I need.
On July 6, 2021 8:10:42 AM GMT+03:00, Michael Snoyman ***@***.***> wrote:
Then I don’t think we can make the change
-- >
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#195 (comment)
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
@hseg @gregwebs I'm not sure if this changes what snoyberg decided, but this comment is in -- | Warning: This module should be considered highly experimental. If the warning is still the case, then it seems like breaking changes are allowed. If not, then I think the comment should be removed. |
I think it should be removed now. To make a breaking change maybe we can make a new module? |
Due to
unions :: (MonoFoldable m, Element m ~ s) => m -> s
, one cannot derive aSetContainer
instance.Specifically, the
Element m ~ s
constraint can't be coerced to the deriving type.I suspect the same is true of the classes in
Data.MonoTraversable
, but haven't confirmed.Notice, too, that as far as I can tell, the default for
unions
isn't overridden in any instances:On github, the only instances for
SetContainer
I could find are https://github.com/AshleyYakeley/Truth/blob/master/shapes/src/Data/FiniteSet.hs, https://github.com/BlackCapCoder/dio/blob/main/src/Algebra/Divisibility.hs, and https://github.com/jnbooth/bitwise-enum/blob/master/Data/Enum/Set/Base.hs, all of which don't override the default.And in
mono-traversable
itself, whetherunions
is overriden is inconsistent, and wherever it is the override is always of formM.unions . otoList
.All of this adds up to suggesting the appropriate definition would be
unions :: Foldable f => f s -> s
, with the only odd cases out being those fromunordered-containers
, who haven't generalizedunions
to arbitraryFoldable
s.And if we're not averse to redefining it, we could just set
unions = Foldable.foldl' union empty
, as is its definition everywhere else.The text was updated successfully, but these errors were encountered: