lspitzner / pqueue

Haskell priority queue package
Other
15 stars 13 forks source link

Please expose .Internals #18

Open Fuuzetsu opened 6 years ago

Fuuzetsu commented 6 years ago

It would be great to have access to some unsafe or just convenient internals. To give two simple examples of each:

Many popular Haskell packages already expose this module. Some choose to not expose it in Haddock documentation (I'd recommend against that). It'd be great if pqueue could as well with the usual disclaimers about no guarantees to the contents.

treeowl commented 2 years ago

I am a little nervous about an Ord-free API, because I want to see if using Ord everywhere will give us useful specialization. Do you have an alternative approach to such specialization?

Fuuzetsu commented 2 years ago

I am a little nervous about an Ord-free API, because I want to see if using Ord everywhere will give us useful specialization. Do you have an alternative approach to such specialization?

I think I don't understand the question. If I am reading my own nearly 4 year old ticket correctly, the Ord-free API would be something in a library somewhere, nothing to do with this package per-se. But I also don't remember what was the exact use-case at the time.

The second point still definitely holds: if I am maintaining my own invariant somewhere about maximal element, it'd be nice to not pay the comparison costs and just insert as-is.

In the end, this ticket is just about exposing some .Internal stuff and you can do whatever you want in there of course, there are no stability guarantees. It's just when I look in the source and see a function doing exactly what I need but can't use it because it's not exported out of the package, it makes me sad.

treeowl commented 2 years ago

@stephen-smith has suggested that a separate internals package is a more principled and usable approach than exposing internals from the main package. I think that's probably a good idea, and I'll try to make it happen.

Fuuzetsu commented 2 years ago

@stephen-smith has suggested that a separate internals package is a more principled and usable approach than exposing internals from the main package. I think that's probably a good idea, and I'll try to make it happen.

I'm not sure what the advantage is except extra maintenance for you but whatever works I suppose. As long as there doesn't end up being scenario of "mypackage-internals" and "mypackage" where the latter also has some own internals that aren't exposed because it's annoying to update mypackage-internals for each change.

lspitzner commented 2 years ago

@stephen-smith has suggested that a separate internals package is a more principled and usable approach than exposing internals from the main package. I think that's probably a good idea, and I'll try to make it happen.

I am not convinced; how exactly would it be more principled and usable? To me it also looks like it adds boilerplate and maintenance burden (version bounds from to -internals etc.) without a justifying gain.

treeowl commented 2 years ago

@lspitzner because the internal and public parts don't necessarily track each other. If we, say, remove the size fields, that would be a major version bump in the internals but a minor one in the public library. If we remove insertBehind (which I definitely think we should), that will be a major version bump to the public library but (likely) wouldn't touch the internals. I've never actually worked with a package that does this, but I think it makes logical sense.

Fuuzetsu commented 2 years ago

Normally you just don't make a major version bump unless you actually change the public API and you just consider .Internals to not be public: it's called .Internals for a reason. There's usually a disclaimer on top of these modules that they can change/break at any time without prior warning and they are not following semver.

Of course the way you're suggesting is also possible – but it's not really what the ticket was originally about as that's a lot of work compared to just moving the module up to exposed modules in cabal file.

treeowl commented 1 year ago

Let's make a separate pqueue-internals package?