CollateX format for GraphML output changed; parser update
[scpubgit/stemmatology.git] / lib / Text / Tradition / Collation.pm
index 3a9b056..68b2adf 100644 (file)
@@ -2,6 +2,7 @@ package Text::Tradition::Collation;
 
 use Encode qw( decode_utf8 );
 use File::Temp;
+use File::Which;
 use Graph;
 use IPC::Run qw( run binary );
 use Text::CSV_XS;
@@ -27,6 +28,7 @@ has 'relations' => (
        handles => {
                relationships => 'relationships',
                related_readings => 'related_readings',
+               get_relationship => 'get_relationship',
                del_relationship => 'del_relationship',
        },
        writer => '_set_relations',
@@ -75,6 +77,12 @@ has 'ac_label' => (
     default => ' (a.c.)',
     );
     
+has 'wordsep' => (
+       is => 'rw',
+       isa => 'Str',
+       default => ' ',
+       );
+    
 has 'start' => (
        is => 'ro',
        isa => 'Text::Tradition::Collation::Reading',
@@ -102,6 +110,12 @@ has 'cached_table' => (
        predicate => 'has_cached_table',
        clearer => 'wipe_table',
        );
+       
+has '_graphcalc_done' => (
+       is => 'rw',
+       isa => 'Bool',
+       default => undef,
+       ); 
 
 =head1 NAME
 
@@ -155,6 +169,9 @@ representing another layer of path for the given witness - that is, when
 a text has more than one possible reading due to scribal corrections or
 the like.  Defaults to ' (a.c.)'.
 
+=item * wordsep - The string used to separate words in the original text.
+Defaults to ' '.
+
 =back
 
 =head1 ACCESSORS
@@ -169,6 +186,8 @@ the like.  Defaults to ' (a.c.)'.
 
 =head2 ac_label
 
+=head2 wordsep
+
 Simple accessors for collation attributes.
 
 =head2 start
@@ -197,10 +216,14 @@ See L<Text::Tradition::Collation::Reading> for the available arguments.
 Removes the given reading from the collation, implicitly removing its
 paths and relationships.
 
-=head2 merge_readings( $main, $second )
+=head2 merge_readings( $main, $second, $concatenate, $with_str )
+
+Merges the $second reading into the $main one. If $concatenate is true, then
+the merged node will carry the text of both readings, concatenated with either
+$with_str (if specified) or a sensible default (the empty string if the
+appropriate 'join_*' flag is set on either reading, or else $self->wordsep.)
 
-Merges the $second reading into the $main one. 
-The arguments may be either readings or reading IDs.
+The first two arguments may be either readings or reading IDs.
 
 =head2 has_reading( $id )
 
@@ -253,12 +276,6 @@ sub BUILD {
 
 ### Reading construct/destruct functions
 
-sub _clear_cache {
-       my $self = shift;
-       $self->wipe_svg if $self->has_cached_svg;
-       $self->wipe_table if $self->has_cached_table;
-}      
-
 sub add_reading {
        my( $self, $reading ) = @_;
        unless( ref( $reading ) eq 'Text::Tradition::Collation::Reading' ) {
@@ -271,7 +288,7 @@ sub add_reading {
        if( $self->reading( $reading->id ) ) {
                throw( "Collation already has a reading with id " . $reading->id );
        }
-       $self->_clear_cache;
+       $self->_graphcalc_done(0);
        $self->_add_reading( $reading->id => $reading );
        # Once the reading has been added, put it in both graphs.
        $self->sequence->add_vertex( $reading->id );
@@ -288,7 +305,8 @@ around del_reading => sub {
                $arg = $arg->id;
        }
        # Remove the reading from the graphs.
-       $self->_clear_cache;
+       $self->_graphcalc_done(0);
+       $self->_clear_cache; # Explicitly clear caches to GC the reading
        $self->sequence->delete_vertex( $arg );
        $self->relations->delete_reading( $arg );
        
@@ -296,15 +314,58 @@ around del_reading => sub {
        $self->$orig( $arg );
 };
 
-# merge_readings( $main, $to_be_deleted );
+=begin testing
+
+use Text::Tradition;
+
+my $cxfile = 't/data/Collatex-16.xml';
+my $t = Text::Tradition->new( 
+    'name'  => 'inline', 
+    'input' => 'CollateX',
+    'file'  => $cxfile,
+    );
+my $c = $t->collation;
+
+my $rno = scalar $c->readings;
+# Split n21 for testing purposes
+my $new_r = $c->add_reading( { 'id' => 'n21p0', 'text' => 'un', 'join_next' => 1 } );
+my $old_r = $c->reading( 'n21' );
+$old_r->alter_text( 'to' );
+$c->del_path( 'n20', 'n21', 'A' );
+$c->add_path( 'n20', 'n21p0', 'A' );
+$c->add_path( 'n21p0', 'n21', 'A' );
+$c->flatten_ranks();
+ok( $c->reading( 'n21p0' ), "New reading exists" );
+is( scalar $c->readings, $rno, "Reading add offset by flatten_ranks" );
+
+# Combine n3 and n4 ( with his )
+$c->merge_readings( 'n3', 'n4', 1 );
+ok( !$c->reading('n4'), "Reading n4 is gone" );
+is( $c->reading('n3')->text, 'with his', "Reading n3 has both words" );
+
+# Collapse n9 and n10 ( rood / root )
+$c->merge_readings( 'n9', 'n10' );
+ok( !$c->reading('n10'), "Reading n10 is gone" );
+is( $c->reading('n9')->text, 'rood', "Reading n9 has an unchanged word" );
+
+# Combine n21 and n21p0
+my $remaining = $c->reading('n21');
+$remaining ||= $c->reading('n22');  # one of these should still exist
+$c->merge_readings( 'n21p0', $remaining, 1 );
+ok( !$c->reading('n21'), "Reading $remaining is gone" );
+is( $c->reading('n21p0')->text, 'unto', "Reading n21p0 merged correctly" );
+
+=end testing
+
+=cut
 
 sub merge_readings {
        my $self = shift;
 
        # We only need the IDs for adding paths to the graph, not the reading
        # objects themselves.
-    my( $kept, $deleted, $combine_char ) = $self->_stringify_args( @_ );
-       $self->_clear_cache;
+    my( $kept, $deleted, $combine, $combine_char ) = $self->_stringify_args( @_ );
+       $self->_graphcalc_done(0);
 
     # The kept reading should inherit the paths and the relationships
     # of the deleted reading.
@@ -322,11 +383,15 @@ sub merge_readings {
        $self->relations->merge_readings( $kept, $deleted, $combine_char );
        
        # Do the deletion deed.
-       if( $combine_char ) {
+       if( $combine ) {
                my $kept_obj = $self->reading( $kept );
-               my $new_text = join( $combine_char, $kept_obj->text, 
-                       $self->reading( $deleted )->text );
-               $kept_obj->alter_text( $new_text );
+               my $del_obj = $self->reading( $deleted );
+               my $joinstr = $combine_char;
+               unless( defined $joinstr ) {
+                       $joinstr = '' if $kept_obj->join_next || $del_obj->join_prior;
+                       $joinstr = $self->wordsep unless defined $joinstr;
+               }
+               $kept_obj->alter_text( join( $joinstr, $kept_obj->text, $del_obj->text ) );
        }
        $self->del_reading( $deleted );
 }
@@ -334,12 +399,12 @@ sub merge_readings {
 
 # Helper function for manipulating the graph.
 sub _stringify_args {
-       my( $self, $first, $second, $arg ) = @_;
+       my( $self, $first, $second, @args ) = @_;
     $first = $first->id
         if ref( $first ) eq 'Text::Tradition::Collation::Reading';
     $second = $second->id
         if ref( $second ) eq 'Text::Tradition::Collation::Reading';        
-    return( $first, $second, $arg );
+    return( $first, $second, @args );
 }
 
 # Helper function for manipulating the graph.
@@ -360,7 +425,7 @@ sub add_path {
        # objects themselves.
     my( $source, $target, $wit ) = $self->_stringify_args( @_ );
 
-       $self->_clear_cache;
+       $self->_graphcalc_done(0);
        # Connect the readings
     $self->sequence->add_edge( $source, $target );
     # Note the witness in question
@@ -381,7 +446,7 @@ sub del_path {
        # objects themselves.
     my( $source, $target, $wit ) = $self->_stringify_args( @args );
 
-       $self->_clear_cache;
+       $self->_graphcalc_done(0);
        if( $self->sequence->has_edge_attribute( $source, $target, $wit ) ) {
                $self->sequence->delete_edge_attribute( $source, $target, $wit );
        }
@@ -410,7 +475,7 @@ be called via $tradition->del_witness.
 sub clear_witness {
        my( $self, @sigils ) = @_;
 
-       $self->_clear_cache;
+       $self->_graphcalc_done(0);
        # Clear the witness(es) out of the paths
        foreach my $e ( $self->paths ) {
                foreach my $sig ( @sigils ) {
@@ -431,12 +496,21 @@ sub add_relationship {
     my( $source, $target, $opts ) = $self->_stringify_args( @_ );
     my( @vectors ) = $self->relations->add_relationship( $source, 
        $self->reading( $source ), $target, $self->reading( $target ), $opts );
-    # Force a full rank recalculation every time. Yuck.
-    $self->calculate_ranks() if $self->end->has_rank;
-       $self->_clear_cache;
+       $self->_graphcalc_done(0);
     return @vectors;
 }
 
+around qw/ get_relationship del_relationship / => sub {
+       my $orig = shift;
+       my $self = shift;
+       my @args = @_;
+       if( @args == 1 && ref( $args[0] ) eq 'ARRAY' ) {
+               @args = @{$_[0]};
+       }
+       my( $source, $target ) = $self->_stringify_args( @args );
+       $self->$orig( $source, $target );
+};
+
 =head2 reading_witnesses( $reading )
 
 Return a list of sigils corresponding to the witnesses in which the reading appears.
@@ -467,13 +541,16 @@ sub reading_witnesses {
 =head2 as_svg( \%options )
 
 Returns an SVG string that represents the graph, via as_dot and graphviz.
-See as_dot for a list of options.
+See as_dot for a list of options.  Must have GraphViz (dot) installed to run.
 
 =cut
 
 sub as_svg {
     my( $self, $opts ) = @_;
+    throw( "Need GraphViz installed to output SVG" )
+       unless File::Which::which( 'dot' );
     my $want_subgraph = exists $opts->{'from'} || exists $opts->{'to'};
+    $self->calculate_ranks() unless $self->_graphcalc_done;
     if( !$self->has_cached_svg || $opts->{'recalc'}    || $want_subgraph ) {        
                my @cmd = qw/dot -Tsvg/;
                my( $svg, $err );
@@ -517,6 +594,7 @@ sub as_dot {
     my $color_common = $opts->{'color_common'} if $opts;
     my $STRAIGHTENHACK = !$startrank && !$endrank && $self->end->rank 
        && $self->end->rank > 100;
+    $STRAIGHTENHACK = 1 if $opts->{'straight'}; # even for subgraphs or small graphs
 
     # Check the arguments
     if( $startrank ) {
@@ -561,7 +639,8 @@ sub as_dot {
        }
        if( $STRAIGHTENHACK ) {
                ## HACK part 1
-               $dot .= "\tsubgraph { rank=same \"#START#\" \"#SILENT#\" }\n";  
+               my $startlabel = $startrank ? 'SUBSTART' : 'START';
+               $dot .= "\tsubgraph { rank=same \"#$startlabel#\" \"#SILENT#\" }\n";  
                $dot .= "\t\"#SILENT#\" [ shape=diamond,color=white,penwidth=0,label=\"\" ];"
        }
        my %used;  # Keep track of the readings that actually appear in the graph
@@ -579,6 +658,8 @@ sub as_dot {
         next if $reading->id eq $reading->text;
         my $rattrs;
         my $label = $reading->text;
+        $label .= '-' if $reading->join_next;
+        $label = "-$label" if $reading->join_prior;
         $label =~ s/\"/\\\"/g;
                $rattrs->{'label'} = $label;
                $rattrs->{'fillcolor'} = '#b3f36d' if $reading->is_common && $color_common;
@@ -640,7 +721,8 @@ sub as_dot {
        }
        # HACK part 2
        if( $STRAIGHTENHACK ) {
-               $dot .= "\t\"#END#\" -> \"#SILENT#\" [ color=white,penwidth=0 ];\n";
+               my $endlabel = $endrank ? 'SUBEND' : 'END';
+               $dot .= "\t\"#$endlabel#\" -> \"#SILENT#\" [ color=white,penwidth=0 ];\n";
        }       
 
     $dot .= "}\n";
@@ -713,14 +795,24 @@ sub _path_display_label {
        }
 }
 
-=head2 witnesses_at_rank
+=head2 readings_at_rank( $rank )
 
-Returns a list of witnesses that are not lacunose, for a given rank.
+Returns a list of readings at a given rank, taken from the alignment table.
 
 =cut
 
-sub witnesses_at_rank {
+sub readings_at_rank {
        my( $self, $rank ) = @_;
+       my $table = $self->alignment_table;
+       # Table rank is real rank - 1.
+       my @elements = map { $_->{'tokens'}->[$rank-1] } @{$table->{'alignment'}};
+       my %readings;
+       foreach my $e ( @elements ) {
+               next unless ref( $e ) eq 'HASH';
+               next unless exists $e->{'t'};
+               $readings{$e->{'t'}->id} = $e->{'t'};
+       }
+       return values %readings;
 }              
 
 =head2 as_graphml
@@ -771,7 +863,8 @@ is( scalar $st->collation->relationships, 3, "Reparsed collation has new relatio
 
 sub as_graphml {
     my( $self ) = @_;
-
+       $self->calculate_ranks unless $self->_graphcalc_done;
+       
     # Some namespaces
     my $graphml_ns = 'http://graphml.graphdrawing.org/xmlns';
     my $xsi_ns = 'http://www.w3.org/2001/XMLSchema-instance';
@@ -784,57 +877,90 @@ sub as_graphml {
     $graphml->setDocumentElement( $root );
     $root->setNamespace( $xsi_ns, 'xsi', 0 );
     $root->setAttributeNS( $xsi_ns, 'schemaLocation', $graphml_schema );
+    
+    # List of attribute types to save on our objects and their corresponding
+    # GraphML types
+    my %save_types = (
+       'Str' => 'string',
+       'Int' => 'int',
+       'Bool' => 'boolean',
+       'RelationshipType' => 'string',
+       'RelationshipScope' => 'string',
+    );
+    
+    # List of attribute names *not* to save on our objects.
+    # We will also not save any attribute beginning with _.
+    my %skipsave;
+    map { $skipsave{$_} = 1 } qw/ cached_svg /;
 
-    # Add the data keys for the graph
+    # Add the data keys for the graph. Include an extra key 'version' for the
+    # GraphML output version.
     my %graph_data_keys;
     my $gdi = 0;
-    my @graph_attributes = qw/ version wit_list_separator baselabel linear ac_label /;
-    foreach my $datum ( @graph_attributes ) {
+    my %graph_attributes = ( 'version' => 'string' );
+       # Graph attributes include those of Tradition and those of Collation.
+       my %gattr_from;
+       my $tmeta = $self->tradition->meta;
+       my $cmeta = $self->meta;
+       map { $gattr_from{$_->name} = 'Tradition' } $tmeta->get_all_attributes;
+       map { $gattr_from{$_->name} = 'Collation' } $cmeta->get_all_attributes;
+       foreach my $attr ( ( $tmeta->get_all_attributes, $cmeta->get_all_attributes ) ) {
+               next if $attr->name =~ /^_/;
+               next if $skipsave{$attr->name};
+               next unless $save_types{$attr->type_constraint->name};
+               $graph_attributes{$attr->name} = $save_types{$attr->type_constraint->name};
+       }
+       
+    foreach my $datum ( sort keys %graph_attributes ) {
        $graph_data_keys{$datum} = 'dg'.$gdi++;
         my $key = $root->addNewChild( $graphml_ns, 'key' );
         $key->setAttribute( 'attr.name', $datum );
-        $key->setAttribute( 'attr.type', $key eq 'linear' ? 'boolean' : 'string' );
+        $key->setAttribute( 'attr.type', $graph_attributes{$datum} );
         $key->setAttribute( 'for', 'graph' );
         $key->setAttribute( 'id', $graph_data_keys{$datum} );          
     }
 
-    # Add the data keys for nodes
+    # Add the data keys for reading nodes
+    my %reading_attributes;
+    my $rmeta = Text::Tradition::Collation::Reading->meta;
+    foreach my $attr( $rmeta->get_all_attributes ) {
+               next if $attr->name =~ /^_/;
+               next if $skipsave{$attr->name};
+               next unless $save_types{$attr->type_constraint->name};
+               $reading_attributes{$attr->name} = $save_types{$attr->type_constraint->name};
+       }
     my %node_data_keys;
     my $ndi = 0;
-    my %node_data = ( 
-       id => 'string',
-       text => 'string',
-       rank => 'string',
-       is_start => 'boolean',
-       is_end => 'boolean',
-       is_lacuna => 'boolean',
-       );
-    foreach my $datum ( keys %node_data ) {
+    foreach my $datum ( sort keys %reading_attributes ) {
         $node_data_keys{$datum} = 'dn'.$ndi++;
         my $key = $root->addNewChild( $graphml_ns, 'key' );
         $key->setAttribute( 'attr.name', $datum );
-        $key->setAttribute( 'attr.type', $node_data{$datum} );
+        $key->setAttribute( 'attr.type', $reading_attributes{$datum} );
         $key->setAttribute( 'for', 'node' );
         $key->setAttribute( 'id', $node_data_keys{$datum} );
     }
 
-    # Add the data keys for edges, i.e. witnesses
+    # Add the data keys for edges, that is, paths and relationships. Path
+    # data does not come from a Moose class so is here manually.
     my $edi = 0;
     my %edge_data_keys;
-    my %edge_data = (
-       class => 'string',                              # Class, deprecated soon
+    my %edge_attributes = (
        witness => 'string',                    # ID/label for a path
-       relationship => 'string',               # ID/label for a relationship
        extra => 'boolean',                             # Path key
-       scope => 'string',                              # Relationship key
-       non_correctable => 'boolean',   # Relationship key
-       non_independent => 'boolean',   # Relationship key
        );
-    foreach my $datum ( keys %edge_data ) {
+    my @path_attributes = keys %edge_attributes; # track our manual additions
+    my $pmeta = Text::Tradition::Collation::Relationship->meta;
+    foreach my $attr( $pmeta->get_all_attributes ) {
+               next if $attr->name =~ /^_/;
+               next if $skipsave{$attr->name};
+               next unless $save_types{$attr->type_constraint->name};
+               $edge_attributes{$attr->name} = $save_types{$attr->type_constraint->name};
+       }
+    foreach my $datum ( sort keys %edge_attributes ) {
         $edge_data_keys{$datum} = 'de'.$edi++;
         my $key = $root->addNewChild( $graphml_ns, 'key' );
         $key->setAttribute( 'attr.name', $datum );
-        $key->setAttribute( 'attr.type', $edge_data{$datum} );
+        $key->setAttribute( 'attr.type', $edge_attributes{$datum} );
         $key->setAttribute( 'for', 'edge' );
         $key->setAttribute( 'id', $edge_data_keys{$datum} );
     }
@@ -850,8 +976,15 @@ sub as_graphml {
     $sgraph->setAttribute( 'parse.order', 'nodesfirst' );
            
     # Collation attribute data
-    foreach my $datum ( @graph_attributes ) {
-       my $value = $datum eq 'version' ? '3.0' : $self->$datum;
+    foreach my $datum ( keys %graph_attributes ) {
+       my $value;
+       if( $datum eq 'version' ) {
+               $value = '3.1';
+       } elsif( $gattr_from{$datum} eq 'Tradition' ) {
+               $value = $self->tradition->$datum;
+       } else {
+               $value = $self->$datum;
+       }
                _add_graphml_data( $sgraph, $graph_data_keys{$datum}, $value );
        }
 
@@ -864,7 +997,7 @@ sub as_graphml {
         my $node_xmlid = 'n' . $node_ctr++;
         $node_hash{ $n->id } = $node_xmlid;
         $node_el->setAttribute( 'id', $node_xmlid );
-        foreach my $d ( keys %node_data ) {
+        foreach my $d ( keys %reading_attributes ) {
                my $nval = $n->$d;
                _add_graphml_data( $node_el, $node_data_keys{$d}, $nval )
                        if defined $nval;
@@ -896,11 +1029,11 @@ sub as_graphml {
                                _add_graphml_data( $edge_el, $edge_data_keys{'extra'}, $aclabel );
                        }
                        _add_graphml_data( $edge_el, $edge_data_keys{'witness'}, $base );
-                       _add_graphml_data( $edge_el, $edge_data_keys{'class'}, 'path' );
                }
        }
        
        # Add the relationship graph to the XML
+       map { delete $edge_data_keys{$_} } @path_attributes;
        $self->relations->_as_graphml( $graphml_ns, $root, \%node_hash, 
                $node_data_keys{'id'}, \%edge_data_keys );
 
@@ -964,7 +1097,7 @@ keys have a true hash value will be included.
 
 sub alignment_table {
     my( $self ) = @_;
-    my $include; # see if we can ditch this
+    $self->calculate_ranks() unless $self->_graphcalc_done;
     return $self->cached_table if $self->has_cached_table;
     
     # Make sure we can do this
@@ -975,9 +1108,6 @@ sub alignment_table {
     my $table = { 'alignment' => [], 'length' => $self->end->rank - 1 };
     my @all_pos = ( 1 .. $self->end->rank - 1 );
     foreach my $wit ( sort { $a->sigil cmp $b->sigil } $self->tradition->witnesses ) {
-       if( $include ) {
-               next unless $include->{$wit->sigil};
-       }
         # print STDERR "Making witness row(s) for " . $wit->sigil . "\n";
         my @wit_path = $self->reading_sequence( $self->start, $self->end, $wit->sigil );
         my @row = _make_witness_row( \@wit_path, \@all_pos );
@@ -1191,7 +1321,17 @@ sub path_text {
        $start = $self->start unless $start;
        $end = $self->end unless $end;
        my @path = grep { !$_->is_meta } $self->reading_sequence( $start, $end, $wit );
-       return join( ' ', map { $_->text } @path );
+       my $pathtext = '';
+       my $last;
+       foreach my $r ( @path ) {
+               if( $r->join_prior || !$last || $last->join_next ) {
+                       $pathtext .= $r->text;
+               } else {
+                       $pathtext .= ' ' . $r->text;
+               }
+               $last = $r;
+       }
+       return $pathtext;
 }
 
 =head1 INITIALIZATION METHODS
@@ -1262,14 +1402,13 @@ my $t = Text::Tradition->new(
 my $c = $t->collation;
 
 # Make an svg
-my $svg = $c->as_svg;
-is( substr( $svg, 0, 5 ), '<?xml', "Got XML doc for svg" );
-ok( $c->has_cached_svg, "SVG was cached" );
-is( $c->as_svg, $svg, "Cached SVG returned upon second call" );
+my $table = $c->alignment_table;
+ok( $c->has_cached_table, "Alignment table was cached" );
+is( $c->alignment_table, $table, "Cached table returned upon second call" );
 $c->calculate_ranks;
-is( $c->as_svg, $svg, "Cached SVG retained with no rank change" );
-$c->add_relationship( 'n9', 'n23', { 'type' => 'spelling' } );
-isnt( $c->as_svg, $svg, "SVG changed after relationship add" );
+is( $c->alignment_table, $table, "Cached table retained with no rank change" );
+$c->add_relationship( 'n24', 'n23', { 'type' => 'spelling' } );
+isnt( $c->alignment_table, $table, "Alignment table changed after relationship add" );
 
 =end testing
 
@@ -1336,14 +1475,20 @@ sub calculate_ranks {
             throw( "Ranks not calculated after $last - do you have a cycle in the graph?" );
         }
     }
-    # Do we need to invalidate the cached SVG?
-    if( $self->has_cached_svg ) {
+    # Do we need to invalidate the cached data?
+    if( $self->has_cached_svg || $self->has_cached_table ) {
        foreach my $r ( $self->readings ) {
-               next if $existing_ranks{$r} == $r->rank;
-               $self->wipe_svg;
+               next if defined( $existing_ranks{$r} ) 
+                       && $existing_ranks{$r} == $r->rank;
+               # Something has changed, so clear the cache
+               $self->_clear_cache;
+                       # ...and recalculate the common readings.
+                       $self->calculate_common_readings();
                last;
        }
     }
+       # The graph calculation information is now up to date.
+       $self->_graphcalc_done(1);
 }
 
 sub _assign_rank {
@@ -1381,6 +1526,13 @@ sub _assign_rank {
     return @next_nodes;
 }
 
+sub _clear_cache {
+       my $self = shift;
+       $self->wipe_svg if $self->has_cached_svg;
+       $self->wipe_table if $self->has_cached_table;
+}      
+
+
 =head2 flatten_ranks
 
 A convenience method for parsing collation data.  Searches the graph for readings
@@ -1391,56 +1543,23 @@ with the same text at the same rank, and merges any that are found.
 sub flatten_ranks {
     my $self = shift;
     my %unique_rank_rdg;
+    my $changed;
     foreach my $rdg ( $self->readings ) {
         next unless $rdg->has_rank;
         my $key = $rdg->rank . "||" . $rdg->text;
         if( exists $unique_rank_rdg{$key} ) {
             # Combine!
                # print STDERR "Combining readings at same rank: $key\n";
+               $changed = 1;
             $self->merge_readings( $unique_rank_rdg{$key}, $rdg );
             # TODO see if this now makes a common point.
         } else {
             $unique_rank_rdg{$key} = $rdg;
         }
     }
-}
-
-=head2 remove_collations
-
-Another convenience method for parsing. Removes all 'collation' relationships
-that were defined in order to get the reading ranks to be correct.
-
-=begin testing
-
-use Text::Tradition;
-
-my $cxfile = 't/data/Collatex-16.xml';
-my $t = Text::Tradition->new( 
-    'name'  => 'inline', 
-    'input' => 'CollateX',
-    'file'  => $cxfile,
-    );
-my $c = $t->collation;
-
-isnt( $c->reading('n23')->rank, $c->reading('n9')->rank, "Rank skew exists" );
-$c->add_relationship( 'n23', 'n9', { 'type' => 'collated', 'scope' => 'local' } );
-is( scalar $c->relationships, 4, "Found all expected relationships" );
-$c->remove_collations;
-is( scalar $c->relationships, 3, "Collated relationships now gone" );
-is( $c->reading('n23')->rank, $c->reading('n9')->rank, "Aligned ranks were preserved" );
-
-=end testing
-
-=cut
-
-sub remove_collations {
-       my $self = shift;
-       foreach my $reledge ( $self->relationships ) {
-               my $relobj = $self->relations->get_relationship( $reledge );
-               if( $relobj && $relobj->type eq 'collated' ) {
-                       $self->relations->delete_relationship( $reledge );
-               }
-       }
+    # If we merged readings, the ranks are still fine but the alignment
+    # table is wrong. Wipe it.
+    $self->wipe_table() if $changed;
 }
        
 
@@ -1466,7 +1585,7 @@ my @common = $c->calculate_common_readings();
 is( scalar @common, 8, "Found correct number of common readings" );
 my @marked = sort $c->common_readings();
 is( scalar @common, 8, "All common readings got marked as such" );
-my @expected = qw/ n1 n12 n16 n19 n20 n5 n6 n7 /;
+my @expected = qw/ n1 n11 n16 n19 n20 n5 n6 n7 /;
 is_deeply( \@marked, \@expected, "Found correct list of common readings" );
 
 =end testing
@@ -1476,6 +1595,8 @@ is_deeply( \@marked, \@expected, "Found correct list of common readings" );
 sub calculate_common_readings {
        my $self = shift;
        my @common;
+       map { $_->is_common( 0 ) } $self->readings;
+       # Implicitly calls calculate_ranks
        my $table = $self->alignment_table;
        foreach my $idx ( 0 .. $table->{'length'} - 1 ) {
                my @row = map { $_->{'tokens'}->[$idx] 
@@ -1509,14 +1630,22 @@ original texts.
 sub text_from_paths {
        my $self = shift;
     foreach my $wit ( $self->tradition->witnesses ) {
-       my @text = split( /\s+/, 
-               $self->reading_sequence( $self->start, $self->end, $wit->sigil ) );
+       my @readings = $self->reading_sequence( $self->start, $self->end, $wit->sigil );
+       my @text;
+       foreach my $r ( @readings ) {
+               next if $r->is_meta;
+               push( @text, $r->text );
+       }
        $wit->text( \@text );
        if( $wit->is_layered ) {
-                       my @uctext = split( /\s+/, 
-                               $self->reading_sequence( $self->start, $self->end, 
-                                       $wit->sigil.$self->ac_label ) );
-                       $wit->text( \@uctext );
+                       my @ucrdgs = $self->reading_sequence( $self->start, $self->end, 
+                                                                                                 $wit->sigil.$self->ac_label );
+                       my @uctext;
+                       foreach my $r ( @ucrdgs ) {
+                               next if $r->is_meta;
+                               push( @uctext, $r->text );
+                       }
+                       $wit->layertext( \@uctext );
        }
     }    
 }
@@ -1543,14 +1672,14 @@ my $t = Text::Tradition->new(
     );
 my $c = $t->collation;
 
-is( $c->common_predecessor( 'n9', 'n23' )->id, 
+is( $c->common_predecessor( 'n24', 'n23' )->id, 
     'n20', "Found correct common predecessor" );
-is( $c->common_successor( 'n9', 'n23' )->id, 
+is( $c->common_successor( 'n24', 'n23' )->id, 
     '#END#', "Found correct common successor" );
 
 is( $c->common_predecessor( 'n19', 'n17' )->id, 
     'n16', "Found correct common predecessor for readings on same path" );
-is( $c->common_successor( 'n21', 'n26' )->id, 
+is( $c->common_successor( 'n21', 'n10' )->id, 
     '#END#', "Found correct common successor for readings on same path" );
 
 =end testing