Hi Pavel,

Currently model class has three selection methods:

def selection(self, selstr, optional=True):
Takes selection string, returns bool selection 

def select(self, selection):
Takes selection (bool or iselection), applies it to self and returns selected part of model object. 

def iselection(self, selstr):
Same as first, but returns iselection.


which is hideously confusing with zero chance to remember, and also inconsistent with the rest.
Let me disagree on the point of inconsistency with the rest. 
selection and iselection functions are available with exactly same meaning for atom_selection_cache.
select is available with exact same meaning (returning part of an object) in hierarchy, af_shared_atom, flex.vec3_double, xray_structure etc which you pointed out. 
  
Can we consolidate all three into one ".select()" that would allow taking bool or int or str?
No, because they are doing different things. Selection and iselection wraps atom_selection_cache functionality and not duplicating those of select() function. ".select()" function already has the described functionality.
 
Or you think there are philosophical arguments against it?
Yes, and practical ones as well. I'm caching atom_selection_cache inside model object, so you don't have to construct it again if you use wrappers available in model class. hierarchy.atom_selection_cache() is not a free function to the point I had to pass the cache around to avoid constructing it again to achieve measurable speedup. Especially for larger models.

At the very least, please use model.get_atom_selection_cache() instead of constructing it.

Best regards,
Oleg Sobolev.
 

Pavel

On 8/16/18 15:00, CCTBX commit wrote:
Repository : ssh://g18-sc-serv-04.diamond.ac.uk/cctbx
On branch  : master



commit c98b44997e8eb7bf53ee828f510b933cd5167758
Author: Oleg Sobolev <osobolev@lbl.gov>
Date:   Thu Aug 16 15:00:20 2018 -0700

    Proper use of model class




c98b44997e8eb7bf53ee828f510b933cd5167758
mmtbx/model/model.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mmtbx/model/model.py b/mmtbx/model/model.py
index 6f2247452..87412f95c 100644
--- a/mmtbx/model/model.py
+++ b/mmtbx/model/model.py
@@ -2312,8 +2312,7 @@ class manager(object):
     sizes = flex.int()
     h = self.get_hierarchy()
     if(macro_molecule_only):
-      asc = h.atom_selection_cache()
-      s = asc.selection("protein or nucleotide")
+      s = self.selection("protein or nucleotide")
       h = h.select(s)
     for r in h.residue_groups():
       sizes.append(r.atoms().size())


_______________________________________________
cctbxbb mailing list
cctbxbb@phenix-online.org
http://phenix-online.org/mailman/listinfo/cctbxbb