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

Add NonEmptyDMap --- depends on #26 #31

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Add NonEmptyDMap --- depends on #26 #31

wants to merge 10 commits into from

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Feb 18, 2019

Need this to curry maps. Example usage forthcoming

CC @cgibbard

Ericson2314 and others added 6 commits January 28, 2019 17:42
Depends on obsidiansystems/dependent-sum#23. Also should
depend on getting rid of GHC <8 support. In this repo.
Do this by factor out just about all explicity recursive DMap functions
into mutually recursive DMap and NonEmptyDMap functions. This largely
deduplicates the implementation of both, while ensuring that neither has
extra indirection/imbalances.
These required creating an `adjustF` function, which was added to both
the empty and non empty map modules for consistency.
@Ericson2314 Ericson2314 changed the title WIP: Add NonEmptyDMap WIP: Add NonEmptyDMap --- depends on #26 Feb 18, 2019
@ryantrinkle
Copy link
Member

This looks like it would impose a very large maintenance overhead going forward; I'm not convinced it's worth it. One very valuable thing about the way this package is currently constructed is that it is nothing more than a simple search-and-replace of containers - I've even considered building it from the containers source using a script. How can we bring down the maintenance cost to something reasonable?

@Ericson2314
Copy link
Member Author

Ericson2314 commented Feb 26, 2019

@ryantrinkle make containers itself do the same thing! :D. In serious, I can make the containers issue now that this exists enough to quantify the overhead and call out the benefits with tree rotations. Hopefully the existing non-empty containers packages are good enough to point to as a need, but I'd make the version of vessel using this too to bolster the argument.

@ryantrinkle
Copy link
Member

Sounds good! I think the main thing is that the containers issue needs to go first; I don't want to merge this here in the hope that we'll eventually become maintainable.

@Ericson2314 Ericson2314 changed the title WIP: Add NonEmptyDMap --- depends on #26 Add NonEmptyDMap --- depends on #26 Feb 27, 2019
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

Successfully merging this pull request may close these issues.

4 participants