AllanK: Does the GroupingStrategy, as it is currently defined, conflate the model and the view? Should model-oriented strategies be orthogonal to view-oriented strategy? It seems to me that COLLAPSING_STRATEGY should be applicable to STACKING_STRATEGY and the other view-oriented strategies. That is, wouldn't one want to be able to collapse and expand a stacked group in the same manner in which you'd want to collapse and expand a non-stacked group?
- Iliana: Yes, you should be able to collapse a stack. See the sample code above.
- AllanK: Then would it really necessary to have a model-oriented option like COLLAPSING_STRATEGY, at least at the model level? That is, if all the other strategies allow collapsing, then do we need a collapsing option? If we didn't, then we could treat all grouping strategies are view-oriented, which might be cleaner. If we did need some model-oriented grouping strategies, then I wonder if it might be cleaner to keep them separated from the view-oriented grouping strategies.
Would having ModelGroupingStrategy and ViewGroupingStrategy interfaces meet your model-view separation? If so, I can't think of how the methods in these interfaces would be different, but for conceptual clarity, we could do this.
AllanK: This is related to the issue above. Should groups be inherently hierarchical? Would that simplify the object model? Are there any situations where you would not want to be able to nest groups? I can certainly see that in the STACKING_STRATEGY people will likely want the ability to hierarchically compose these stacks and have the ability to alternative vertical and horizontal partitioning.
- Iliana: Nested grouping could be done like this:
// Example 2. Creating nested groups
CyNetwork subgroup = methodThatGetsASubgroup();
CyNetwork [] groups = groupManager.getGroupsInNetwork(myNet);
Map args = new Hashtable();
for(int i = 0; i < groups.length; i++){
if(groups[i].getIdentifier().equals(SOME_NET_ID)){
groupManager.addGroupToNetwork(groups[i], subgroup); // nest a group
// recursive collapse:
args.put(CollapsingStrategy.IS_RECURSIVE, Boolean.TRUE);
collapsingStrategy.group(myNet,groups[i],args); // collapses the nested group
// OR, stack subsubgroup, collapse subgroup:
args.put(CollapsingStrategy.IS_RECURSIVE, Boolean.FALSE);
stackingStrategy.group(groups[i],subgroup); // stack nested group
collapsingStrategy.group(myNet,groups[i],); // collapse top level group
}
}
// This is code in a GroupingStrategy:
public boolean group (CyNetwork network, CyNetwork subnetwork, Map args){
if(args.containsKey(IS_RECURSIVE) && args.get(IS_RECURSIVE).equals(Boolean.TRUE)){
// recursive grouping, use GroupManager.getGroupsInNetwork to get groups within groups...
CyNetwork [] subsubgroups = groupManager.getGroupsInNetwork(subnetwork);
// .. do stuff
}else{
// non-recursive, do stuff...
}
}AllanK: I wonder if the spelling of the method getGroupedInNetworks() may be a little awkward? Should we use an alternative spelling such as getContainingNetworks()?
Iliana: I changed it to getContainingNetworks(). Thanks.
AlexP: The thorough documentation makes this all much more transparent. Thanks, Iliana. I like the more generic "grouping" terminology. It fits well with the limited goals of metanodes in 2.3. Concerning theGroupingManagerand the issue of data structure, I am not expert enough to have a full vote, but your reasoning seems sound (e.g., hidden is safer).
- Kristina: I am wondering how the three different data structures mentioned above would each work in coloring the metanode based on the underlying child nodes. If there is for example expression data available for each of the nodes in the metanode, then we would want the metanode to be colored based on these colors, for example in a striped manner or with the average color. It is not clear to me if the two data structures mentioned first (which require a node be created) would pose a problem in this kind of coloring.
MikeSmoot: I think GroupingStrategy.degroup(*) methods should be renamed ungroup. Group/Ungroup is the terminology used by Photoshop and others for collecting graphical entities into a single object, which is pretty similar to what we're trying to do.
IlianaAvila - Done. Thanks for the suggestion.
MikeSmoot: Do we really need two GroupingManager.addGroupToNetwork methods? Do we expect it to be common to add an array of networks to another? Will there be anything to the (array) method beyond a for loop that just calls the other addGroupToNetwork? If we think this action will be common, by which I mean the primary way people will interact with the interface, then I think it's ok to keep. On the other hand, if this method will only be called in special circumstances, then I say remove it as it's easy enough to write a for loop. Just trying to keep things as simple and complete as possible.
IlianaAvila - the name of the method is misleading. I have changed it to: GroupManager.tagSubnetworkAsGroup(CyNetwork net,CyNetwork subnet). It does not actually add any nodes/edges to a network. It just tags the subnetwork as a group in a given network.
MikeSmoot: Actually, my point wasn't about the name but that we have two methods that (apparently) do exactly the same thing with the only difference being that one operates on a single subnet, while the other operates on an array of subnets. If we expect the implementation of GroupManager.tagSubnetworkAsGroup(CyNetwork net,CyNetwork[] subnet) to look like:
public void tagSubnetworkAsGroup(CyNetwork net,CyNetwork[] subnet) {
for ( int i = 0; i < subnet.length; i++ )
tagSubnetworkAsGroup(net,subnet[i]);
}- then I think we should remove it to keep things simple.