-
Notifications
You must be signed in to change notification settings - Fork 16
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
Adding Fix to base #190
Comments
|
No, that was a mistake. I've fixed the definition to use the newtype. |
The For all the variations of |
|
|
Can you say what the benefit of having this in |
If I have to put things in order, probably: 3 (not aware), and then 1 (don't want a dependency) because of 2 (it's trivial to redefine). I don't think there are other good reasons to not depend on EDIT: historical note, |
The
Again, by having it in base we have a blessed definition of Fix to use across libraries and packages, that doesn't incur in a dependency and is uniform across users :) |
Hmm, so far, the Annoying that there is no Can we add it to |
Probably. That said, adding |
`
I agree, i'd expect type family Fix f where
Fix f = f (Fix f) to live there. |
It's a class FFunctor t where
ffmap :: (forall x. f x -> g x) -> t f -> t g EDT: actually no, |
I still haven't thought of a good module namespace for Another idea is Then again, it's also a bit limiting to say |
What about |
I like this suggestion! Also, to add my two cents, I like the idea that commonly used data types are in base. For the reasons @alt-romes mentioned. |
I'm torn. At one hand, I do want On the other hand, if people are not inclined to use If The proposal should list everything that will be included. I'm okay with the @alt-romes Could you update your proposal to specify the new API additions precisely? What types, instances and functions do you propose to add? If you have the capacity, it would be quite beneficial to look at other libraries and find the common parts they reimplement (in terms of functions and instances). At the same time, I feel like we need more feedback. @andrewthad @batterseapower @dreixel @wrengr @sellout @folivetti @sebastiaanvisser @brendanhay @gspia Could you share (if possible) what stopped you from using the And would you reconsider reusing the type if |
Maybe I'm boring, but I've never used it. As such I have no strong opinion either way, but would like to see more evidence that this is the case, perhaps. |
I would prefer a moratorium on adding misc new things to What is wrong with simply encouraging all those other libraries to depend on |
I'm generally in agreement @Ericson2314, and am usually on your side of that field. However, there's a balance to be had regarding postponing improvements to the standard library/ecosystem now for the promise of a (distant) future in which it will be simpler to do so; and I think this is one of the cases in which the addition is minimal in contrast to the benefits to be gained. I'm motivated to see this through because I think the Fix-using ecosystem is fragmented in the sense that the definitions of I don't think encouragement for libraries to use
I'd likewise like to hear from other library developers. Note: I've voiced my desire to add |
@chshersh I've added the "The Specific Changes" sections with what I specifically propose doing. |
If it's going to be in I would be more open if there were a need in |
I think it would most valuable if we had input from other maintainers using Nonetheless, please indulge me... As I see it, an interoperability problem can be a Also consider that not all programs can depend on |
Given the trivial dependency footprint of Having canonical packages for these types also means you can usually get instances/utility functions right, once, for everyone. Waiting for a GHC release to add "just one thing" is never fun, because you often can't actually use it without CPP for another few versions anyway. |
I generally agree that it's preferrable to just have libraries depend on |
I wonder if there is a way to avoid having such long discussion about general policies (should something be in base or in it's own library?) for each case anew. Are there clear criteria that can be written down from which 80% of these proposals can be judged again? Arguments “its easier to find, small dependencies are good/bad, it helps the ecosystem to convergen” are not particularly specific to |
I tried and it went nowhere: #141 |
The GHC test suites can do as they please. They are less concerned with ecosystem interoperability and more with making sure things don't break between releases. It's up to the GHC developers to decide if the redefinition of
It's becoming more difficult to believe that you're serious. I quote myself: #143 (comment) |
Yes, the experience @endgame and I had with IMO CLC members / hackage / someone lightly suggesting dependencies to avoid this sort of dedup and consolidating and canonical tiny little libraries that do one boring thing is good. The smaller the library, the easier it is to have a clear winner with no one's ego hurt. |
I'm serious about the proposal, despite the light-hearted tone @mixphix. The GHC testsuite is anecdotal evidence of a situation which I think can be improved. My personal motivation is from witnessing a clash between (Unfortunately my own library can't depend on non-boot libraries, as one of its goals is integration in GHC -- not that it matters, I'm happy to be the only I'll be happy to forfeit this if no one else is in support of the proposal. Let's just wait a few days. |
I think this proposal would be a lot easier to justify if Package maintainers may say "Oh, I would have used that if I'd known about it," which is fine, but there's possibly a reason why they don't use it - and patching libraries to use |
I don't necessarily agree with this. I don't see much benefit in having |
So, the Problem here is that we have a type that's defined in a bunch of places, and we want those libraries to depend on an upstream version that can be shared. This makes sense to me. But the first step, in my mind, is actually ensuring that the upstream version can be shared, and the demonstration is actually sharing that upstream version. Once we have a The primary benefit to moving |
As a heavy and long-term user of @alt-romes the strongest motivation to move @phadej what do you think as a maintainer of |
tl;dr – I don’t support moving this to base. I’m mentioned above as one of the authors of a package that defines its own My library ( I if the types are added to And if Also, the properties of recursion schemes only hold if the pattern functor is strict. So there is already complexity in shadowing Also, as I developed This is just my stance, as an author of a “competing” package, but hopefully it adds some context around the implementations of these things not being settled (again, even if the types are identical, it’s good that they’re distinct). |
Thanks for your perspective on this @sellout. As @Bodigrim also pointed out, the functions associated with Thank you both for joining the discussion too. |
This is a fully general argument for including every type ever defined in |
You're right. Apologies, I withdraw that argument. |
Don't get me wrong, I think that argument is compelling in some cases and not in others, but I don't have a good sense of where the boundaries are. In some sense, you can think of the types in I personally believe I don't think it's the right answer, but as a thought experiment: would |
I think it would be easier to swap dependency order between |
Yes, that's a reasonable idea. My motivation is also the redefinition of Fix across multiple packages, and thinking it is ultimately a "core" data type that could be available in the standard library (as is in the proposal description). Otherwise, I think the discussion has come to a halt. Could you trigger a vote @Bodigrim ? Thank you |
@alt-romes we need a specific MR to vote on. Also, while it's not mandatory, I imagine it would help the CLC members to decide, if there was a migration plan: what and when happens to |
Most likely if the patch is something else than moving But you probably want to have Technically CLC doesn't need to consider |
Technically true, but practically it defeats the purpose of unifying the ecosystem around the same type. For instance, I don't see myself supporting the addition unless you are happy to migrate to it. |
I don't know whether I'm happy or not. There isn't anything concrete in the proposal. (vs. migrate |
Oh, I see. I am also happy to change the proposal to "Move Data.Fix to base", if both @phadej and the CLC were happy with that too. My concern there is for @phadej, since maintaining Data.Fix in base is much more burdensome than maintaining it in That said, |
@alt-romes the question of instances arises in both scenarios: if we move a data type, we should include instances as well. Currently instance Eq1 f => Eq (Fix f)
instance Ord1 f => Ord (Fix f)
instance Show1 f => Show (Fix f) Recently instance Eq (f (Fix f)) => Eq (Fix f) or instance (forall a. Eq a => Eq (f a)) => Eq (Fix f) The question is whether we should change this as a part of migration into In any case, it would be helpful to have an MR to discuss. |
If TL;DR moving |
Tangentially related, one thing I was always wondering about is whether it's possible to explain to GHC that |
@Bodigrim kind of, but no. {-# LANGUAGE QuantifiedConstraints #-}
import Data.Coerce
import Data.Type.Coercion
newtype Fix f = Fix (f (Fix f))
newtype Fix' f = Fix' (f (Fix' f))
coe :: (forall a b. Coercible a b => Coercible (f a) (f b))
=> Fix f -> Fix' f
coe = coerce
We can encode EDIT: and there cannot be general recursion, imagine if unsafe :: a :~: b
unsafe = unsafe just worked. Not good. ( |
This is nowhere near to a long discussion ;) It's difficult to write an actionable policy of this kind. E. g., such policy would most likely advise against #167 (a function is an More generally, my view is that CLC should be "politically neutral". A particular cast of CLC can potentially (with a lot of effort!) compose a collective manifesto - just to be discarded as soon as another member joins or leaves. That's why I don't believe such effort will be well spent. Sorry for digression. |
@alt-romes if there is no progress on preparing an MR for discusssion, I'll close as abandoned in two weeks. |
Closing as abandoned. Feel free to reopen once you get time to prepare an MR. |
Thanks @Bodigrim, sorry for my non-response. I am incredibly busy finishing my master thesis. I hope to get back to this after that is done. Thanks again. |
Proposal
This is a proposal to add the
Fix
datatype to base.Using the constructors from
data-fix
,Fix
is defined as:Fix
is a useful, well-known and widely used datatype.However, this datatype seems to be redefined times and times again across different packages.
Here's a small list that we ought to increment to motivate the proposal further:
The last two are (1) a library I wrote that defines Fix and (2) a library that @folivetti wrote that depends on (1) and also defines Fix -- and has to convert back and forth between the (1) Fix and the (2) Fix.
In light of the common need for this datatype, and to avoid duplicating the definition across users, I propose providing
Fix
in some module in base (Data.Fix
would be nice, but that's definitely taken, although...).Edit: As @phadej mentioned, this also means different packages can provide instances for Fix, like
aeson
.Additional considerations
It could be the case that some of these packages do want their own variant of
Fix
, or for it to instance classes differently, or to have different documentation (though I suppose that could be arranged in the re-exports).We shouldn't add this to Prelude
Fix
looks like a useful concept to teach, and having to hide it from Prelude before showing the definition is anticlimaticBreakage
This is potentially a breaking change, depending on where we define
Fix
, since all the packages defining Fix can clash with the base definition. However, this is intended breakage. Ideally, these definitions would be dropped in favor of theFix
in base.Not all
Fix
definitions are equal (some useout
,MkFix
,unFix
, etc...), so it might not be as trivial as swapping the datatype for some packages.Alternatives
An alternative to defining just the
Fix
datatype in some module that cannot clash withData.Fix
in base, we could /mergedata-fix
/ altogether, and moveData.Fix
to base.That has the added benefit of promoting use of recursion schemes, since they're provided in base :)
The Specific Changes
Specifically, I propose to add exactly
to a module that avoids clashes (e.g.
Data.Recursion.Fix
,Data.Fix.Type
).I don't propose anything else. It's not clear which functions we should add together with the definition, under what names (
cata
vsfold
), and it's not bad if functions are duplicated across packages -- as long as the data type they work on is the same. We can consider adding more things besidesFix
, but I'd rather keep the proposal as smallest. Additions can be done incrementally by someone with better motivation to do so.EDIT: We should also consider which instances to give the type, and which documentation. I would appreciate advice on this. Perhaps using the instances from
data-fix
is a good plan.The text was updated successfully, but these errors were encountered: