Have and expose .Internal modules

[rpglover64]

containers does it.

Today, my co-worker had a problem to which my solution was alterF; unfortunately, alterF is not yet in the released version. If the internals had been exposed, we could have backported the code from the github version without any version shenanigans; as it was, we had to accept the (probably minor) performance hit of lookup + insert due to the retraversal.

[sjakobi]

Related: #103, #98

[sjakobi]

I'd like to prioritize this issue since there's so much demand for access to the internals (see also #25, #103).

What should the module hierarchy for the internals look like? How about

• Data.HashMap.Internal for parts of Data.HashMap.Base
• Data.HashMap.Internal.Strict for parts of Data.HashMap.Strict.Base
• Data.HashMap.Internal.Array for parts of Data.HashMap.Array (if there's demand)

?

If we want to expose the constructors in a PVP-versioned module as I proposed in #98 (comment), that could happen in Data.HashMap.Types or Data.HashMap.Internal.Types.

@emilypi, @treeowl Thoughts?

[jamespayor]

I too am encountering a longing for more of the internals, often when I want some inner-loop code doing a specialized thing to be able to run quickly. For instance, today I wanted to be able to grab a list of the items and their hashes from a HashSet, and whilst that data is in there it is inaccessible. For now, I settle for either recomputing the hashes, or storing a HashSet (Hashed a)...

(Some fuller notes from me in #280, which was just a duplicate of this issue.)

---

My take on exported vs hidden modules is that probably everything should be exported. I think it's fair game to have some of the "exported" modules be subject to arbitrary changes patch by patch, and it seems better to me to allow users this access (given that they know what they're getting into) than to attempt to gatekeep.

That would suggest a fairly simple change in which all the modules become exposed, but some are considered internal and not public facing.

Falling short of that, at least exposing the constructors would allow people to do what they need to. I do like the sound of #98 (comment), in which some subset of the internals (such as the data types) do have versioning guarantees.

[sjakobi]

Thanks for the feedback, @jamespayor! :)

I agree that it would be best and easiest to simply expose all the internal modules. I think we should expose them with different names though to make it clearer what's internal and what isn't. How about

Data.HashMap.Internal        <- Data.HashMap.Base   # or Data.HashMap.Internal.Base?
Data.HashMap.Internal.Array  <- Data.HashMap.Array
Data.HashMap.Internal.List   <- Data.HashMap.List
Data.HashMap.Internal.Strict <- Data.HashMap.Strict.Base
Data.HashMap.Internal.Unsafe <- Data.HashMap.Unsafe
Data.HashSet.Internal        <- Data.HashSet.Base

?

I believe that in order to avoid creating merge conflicts for the large number of open PRs, we should probably not move the existing internal modules, but instead add modules with the new names that reexport the existing internal modules.

[treeowl]

The word Base usually goes away in favor of Internal.

…

On Thu, Jul 2, 2020, 7:54 AM Simon Jakobi ***@***.***> wrote: Thanks for the feedback, @jamespayor <https://github.com/jamespayor>! :) I agree that it would be best and easiest to simply expose all the internal modules. I think we should expose them with different names though to make it clearer what's internal and what isn't. How about Data.HashMap.Internal <- Data.HashMap.Base # or Data.HashMap.Internal.Base? Data.HashMap.Internal.Array <- Data.HashMap.Array Data.HashMap.Internal.List <- Data.HashMap.List Data.HashMap.Internal.Strict <- Data.HashMap.Strict.Base Data.HashMap.Internal.Unsafe <- Data.HashMap.Unsafe Data.HashSet.Internal <- Data.HashSet.Base ? I believe that in order to avoid creating merge conflicts for the large number of open PRs, we should probably not move the existing internal modules, but instead *add* modules with the new names that reexport the existing internal modules. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#211 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOOF7OUTGM5T5MA73BHBJTRZRYQ3ANCNFSM4FDPHW6Q> .

[jamespayor]

Cool - yeah renaming to Internal in that fashion seems correct, and checks my boxes.

@sjakobi fwiw, on my model of Git, if it's just renaming the files then the merge will likely go smoothly. If you do try renaming them, you could checkout the most complicated PR and verify that a merge or rebase works. If so, we get to avoid duplication in the file structure.