-
Notifications
You must be signed in to change notification settings - Fork 115
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
specifying a new Tree API #83
Comments
My current thoughts on this are as follows:
I believe we should refer to Guava's BinaryTreeTraverser for good (if not minimal) initial inspiration on this. For example, it seems obvious to me that we'd want specific methods for obtaining the left child and right child of a particular node. However, it's not so clear to me what "child" would mean for edges in NetworkTrees/TreeNetworks; perhaps successors for directed TreeNetworks, but for undirected ones it's not so clear to me.
I cannot think of alternatives for "TreeNetwork"/"NetworkTree" and "ValueTree" ATM, so I'd personally be happy with "NetworkTree" to be consistent with "ValueTree". |
Ah, it occurs to me that directed edges would only really have one child each, and undirected edges would potentially have two children or no children (depending on how we'd want to define "children"), since edges can only have 2 associated nodes IIUC - no more, no less. |
And it also occurs to me that Trees are inherently directed, so having undirected edges in a NetworkTree wouldn't make much sense... |
So, funny story about That doesn't mean that we might not want a binary (or k-ary) tree implementation, but I'm not yet 100% convinced it's worth it. (It may be worth looking at the Tree types in the common.graph branch to see what the current state of things is.)
I would not be inclined to define "child" for an edge. A child is just a particular kind of successor. |
Sounds like a rather good reason to me then to leave binary or k-ary trees to one side for now and perhaps only work on them if someone requests them? But having said that, I'm kind of curious how you came to the conclusion that no-one uses BTT externally. Would you be able to shed some light on this? |
I agree that while it may be useful to think about how binary/k-ary trees would fit into the type system, we probably don't need to actually write code to support them absent some requests to that effect. How I came to the conclusion that noone uses BTT externally is that I did some analysis on this dataset: https://cloud.google.com/bigquery/public-data/github It's obviously not a complete picture, but I'm willing to take that data set as being a reasonable proxy for "everyone" given its size. There were only three distinct uses of BTT in that entire data set, and that none of them actually required BTT (TreeTraverser would have done just fine; the only thing that BTT gives you is the ability to do inorder traversal). |
Oh, and a brief placeholder thought for later: I'm not sure that there's much value in a Network variant of a Tree. If one wants an object associated with each edge, ValueGraph does that just fine. Might be worth waiting to see if someone asks for a TreeNetwork. |
Another issue we're going to need to address here : make sure that the Tree classes are tested as well as the Guava common.graph classes (and not just indirectly via TreeUtils, etc.). |
@jrtom I'm happy to have a go at increasing test coverage for |
@jrtom Actually it occurs to me again that Guava's tests are licensed under the Apache 2.0 license, whereas JUNG is licensed under one of the BSDs. Would you prefer that I do the tests from scratch, or are you happy that I take ideas from Guava's tests? |
@jbduncan The licensing requirements of Apache 2.0 and the 3-clause BSD license (which JUNG uses) are compatible, as I understand it, so I don't think that taking inspiration from Guava's tests should be a problem (although we always meant to reconsider how they were put together given time). |
@jrtom I admit that I'm struggling to understand what you mean when you say, "(although we always meant to reconsider how they were put together given time)". Would you mind clarifying for me what you meant by "they"?
SGTM. :) |
I apparently missed the above comment. What I meant by "they" was the common.graph unit tests. I mean, they're not terrible, but I remember a discussion from a few years ago on how to refactor them to reduce redundancy (and make it easier for others to reuse the code). |
Not sure why GitHub thinks this issue should have been closed, there are still unresolved issues here, such as: Side note: there's been internal discussion of making public the |
In the common.graph branch (the precursor to JUNG 3.0) there are now some types for supporting tree structures: https://github.com/jrtom/jung/tree/common.graph/jung-api/src/main/java/edu/uci/ics/jung/graph
*CTree*
are placeholder names.In that branch, the
Tree
types and implementations have been removed, so those names are now available. (I'm not worried about breaking anyone by reusing those types, because JUNG 3.0 has already burned that barn behind us, as it were: moving from JUNG 2.x to JUNG 3.x will require a migration from existing users.)Points to consider:
The text was updated successfully, but these errors were encountered: