Skip to content
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

Open
hseg opened this issue Jul 5, 2021 · 6 comments
Open

SetContainer is not derivable #195

hseg opened this issue Jul 5, 2021 · 6 comments

Comments

@hseg
Copy link

hseg commented Jul 5, 2021

Due to unions :: (MonoFoldable m, Element m ~ s) => m -> s, one cannot derive a SetContainer 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, whether unions is overriden is inconsistent, and wherever it is the override is always of form M.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 from unordered-containers, who haven't generalized unions to arbitrary Foldables.
And if we're not averse to redefining it, we could just set unions = Foldable.foldl' union empty, as is its definition everywhere else.

@snoyberg
Copy link
Owner

snoyberg commented Jul 6, 2021

I'd rather not have a breaking API change, but I'm open to a PR that keeps backwards compatibility.

@hseg
Copy link
Author

hseg commented Jul 6, 2021 via email

@snoyberg
Copy link
Owner

snoyberg commented Jul 6, 2021

Then I don’t think we can make the change

@hseg
Copy link
Author

hseg commented Jul 6, 2021 via email

@BebeSparkelSparkel
Copy link
Collaborator

@hseg @gregwebs I'm not sure if this changes what snoyberg decided, but this comment is in Data.Containers

-- | 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.

@gregwebs
Copy link
Collaborator

I think it should be removed now.

To make a breaking change maybe we can make a new module?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants