search

salilab / imp

The Integrative Modeling Platform
https://integrativemodeling.org
GNU General Public License v3.0
73 stars 30 forks source link

show_molecular_hierarchy() should also show representations #929

Closed cgreenberg closed 8 years ago

cgreenberg commented 8 years ago

Technically they're not in the hierarchy, but I think it's a good idea. Alternatively we could add a flag (show_representations=false) or even a different function like show_representations(). What do you think @benmwebb ?

benmwebb commented 8 years ago

My vote is for adding a new function. That's a big behavior change to make to an existing function. Of course, crosslink the two functions in the docs so each is easy to find.

cgreenberg commented 8 years ago

I'm not sure it's such a big change to just have show() recurse through the Representations, I mean that's exactly what rmf_show does.

BTW I cannot find in the code where it actually recurses down the tree, atom::Hierarchy::show() doesn't do that....

benmwebb commented 8 years ago

Look at the code for IMP::core::HierarchyVisitor.

cgreenberg commented 8 years ago

Oh crap I see show_molecular_hierarchy actually calls core::Hierarchy:show() which recurses with the kernel internal macro IMP_PRINT_TREE...damn ok I'm not going near crazy. Why doesn't that use HierarchyVisitor and breadth_first_search instead of the macro?

benmwebb commented 8 years ago

Good question. Daniel 1.0 loved macros though. But if you can make your new code work with HierarchyVisitor, by all means replace it.

cgreenberg commented 8 years ago

OK so this won't work - the visitor just does something to a node as you traverse down the tree with get_child(i). I actually need a function that searches through children AND representations (and their children). I don't see how to use any existing code to do that. That's ok though.

cgreenberg commented 8 years ago

One option is to actually use IMP_PRINT_TREE but this is painful because I'd have to create a new function Hierarchy::get_child_then_representations(i) which based on i gives either a child or a representation item. Or I could just kinda copy how the macro does stuff...