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

A few more extractors for Data.Tree #140

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

A few more extractors for Data.Tree #140

wants to merge 1 commit into from

Conversation

athanclark
Copy link

I've found some use-cases for looking at all the "paths" that a tree has within it - both "complete" paths (all transitively connected nodes from the root to the end-points), and "partial" paths (all steps in-between on the paths).

The two functions I've added shouldn't clash with other namespaces easily, and they're simple enough to see they are correct. I haven't added any quickcheck tests or criterion benchmarks yet, please let me know if you'd like me to do this.

Thank you for maintaining this beast of a library! I'd also like to fulfill some of the wishes in #39 later if possible.

@foxik
Copy link
Contributor

foxik commented Mar 6, 2015

Hello,

thanks for spending time improving containers.

Nevertheless, as noted in the README, please note that all API enhancements should be discussed on libraries@haskell mailing list. Please follow https://wiki.haskell.org/Library_submissions#Guide_to_proposers and start a discussion there.

@athanclark
Copy link
Author

I apologize! I'll present these ideas to the list and keep this thread up-to-date on any progress. Sorry about this.

@foxik
Copy link
Contributor

foxik commented Mar 7, 2015

Do not apologize, it is perfectly fine. The process of public discussion is quite unexpected (at least it was for me when I started with this).

However, it allows people to be involved in API design of widely used packages (or at least monitor it) and it makes the API changes better (people can discuss proper names, parameters, consistency with other similar libraries [that is not the case with Tree, but is for Map etc. where we have {,Int}{Set,Map} here and also Hash{Set,Map} in unordered-containers).

@treeowl
Copy link
Contributor

treeowl commented Apr 13, 2016

@athanclark, how was your proposal received on the list?

@treeowl
Copy link
Contributor

treeowl commented May 22, 2016

I also have some doubts about the efficiency of these. Might it be better to form the paths backwards and then reverse them?

@recursion-ninja
Copy link

Wouldn't forming the paths backwards and later reversing them greatly hinder laziness and make the operations on infinite trees impossible?

@treeowl
Copy link
Contributor

treeowl commented May 11, 2017

@recursion-ninja, that's a problem, for sure, but I think there's already a problem. Look at the definition of paths.

paths (Node a as) = map (a:) $ concatMap paths as

The first element of the result is strict in the first element of concatMap paths as. You end up recursively descending the whole tree to get the first element of the result, if I'm not mistaken. There may be a way around this; I haven't dug into the details.

@treeowl
Copy link
Contributor

treeowl commented Jan 22, 2018

@athanclark Are you still hoping to get some of these in? I haven't seen a libraries proposal or any progress on efficient implementation.

Copy link
Member

@sjakobi sjakobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT @treeowl's concerns need to be addressed.

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

Successfully merging this pull request may close these issues.

None yet

5 participants