start using witness->text and ->layertext for consistency checking
[scpubgit/stemmatology.git] / lib / Text / Tradition / Collation.pm
index 6ae4f2d..10d215e 100644 (file)
@@ -65,7 +65,7 @@ has 'linear' => (
     isa => 'Bool',
     default => 1,
     );
-
+    
 has 'ac_label' => (
     is => 'rw',
     isa => 'Str',
@@ -86,20 +86,146 @@ has 'end' => (
        weak_ref => 1,
        );
 
-# The collation can be created two ways:
-# 1. Collate a set of witnesses (with CollateX I guess) and process
-#    the results as in 2.
-# 2. Read a pre-prepared collation in one of a variety of formats,
-#    and make the graph from that.
-
-# The graph itself will (for now) be immutable, and the positions
-# within the graph will also be immutable.  We need to calculate those
-# positions upon graph construction.  The equivalences between graph
-# nodes will be mutable, entirely determined by the user (or possibly
-# by some semantic pre-processing provided by the user.)  So the
-# constructor should just make an empty equivalences object.  The
-# constructor will also need to make the witness objects, if we didn't
-# come through option 1.
+=head1 NAME
+
+Text::Tradition::Collation - a software model for a text collation
+
+=head1 SYNOPSIS
+
+  use Text::Tradition;
+  my $t = Text::Tradition->new( 
+    'name' => 'this is a text',
+    'input' => 'TEI',
+    'file' => '/path/to/tei_parallel_seg_file.xml' );
+
+  my $c = $t->collation;
+  my @readings = $c->readings;
+  my @paths = $c->paths;
+  my @relationships = $c->relationships;
+  
+  my $svg_variant_graph = $t->collation->as_svg();
+    
+=head1 DESCRIPTION
+
+Text::Tradition is a library for representation and analysis of collated
+texts, particularly medieval ones.  The Collation is the central feature of
+a Tradition, where the text, its sequence of readings, and its relationships
+between readings are actually kept.
+
+=head1 CONSTRUCTOR
+
+=head2 new
+
+The constructor.  Takes a hash or hashref of the following arguments:
+
+=over
+
+=item * tradition - The Text::Tradition object to which the collation 
+belongs. Required.
+
+=item * linear - Whether the collation should be linear; that is, whether 
+transposed readings should be treated as two linked readings rather than one, 
+and therefore whether the collation graph is acyclic.  Defaults to true.
+
+=item * baselabel - The default label for the path taken by a base text 
+(if any). Defaults to 'base text'.
+
+=item * wit_list_separator - The string to join a list of witnesses for 
+purposes of making labels in display graphs.  Defaults to ', '.
+
+=item * ac_label - The extra label to tack onto a witness sigil when 
+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.)'.
+
+=back
+
+=head1 ACCESSORS
+
+=head2 tradition
+
+=head2 linear
+
+=head2 wit_list_separator
+
+=head2 baselabel
+
+=head2 ac_label
+
+Simple accessors for collation attributes.
+
+=head2 start
+
+The meta-reading at the start of every witness path.
+
+=head2 end
+
+The meta-reading at the end of every witness path.
+
+=head2 readings
+
+Returns all Reading objects in the graph.
+
+=head2 reading( $id )
+
+Returns the Reading object corresponding to the given ID.
+
+=head2 add_reading( $reading_args )
+
+Adds a new reading object to the collation. 
+See L<Text::Tradition::Collation::Reading> for the available arguments.
+
+=head2 del_reading( $object_or_id )
+
+Removes the given reading from the collation, implicitly removing its
+paths and relationships.
+
+=head2 merge_readings( $main, $second )
+
+Merges the $second reading into the $main one. 
+The arguments may be either readings or reading IDs.
+
+=head2 has_reading( $id )
+
+Predicate to see whether a given reading ID is in the graph.
+
+=head2 reading_witnesses( $object_or_id )
+
+Returns a list of sigils whose witnesses contain the reading.
+
+=head2 paths
+
+Returns all reading paths within the document - that is, all edges in the 
+collation graph.  Each path is an arrayref of [ $source, $target ] reading IDs.
+
+=head2 add_path( $source, $target, $sigil )
+
+Links the given readings in the collation in sequence, under the given witness
+sigil.  The readings may be specified by object or ID.
+
+=head2 del_path( $source, $target, $sigil )
+
+Links the given readings in the collation in sequence, under the given witness
+sigil.  The readings may be specified by object or ID.
+
+=head2 has_path( $source, $target );
+
+Returns true if the two readings are linked in sequence in any witness.  
+The readings may be specified by object or ID.
+
+=head2 relationships
+
+Returns all Relationship objects in the collation.
+
+=head2 add_relationship( $reading, $other_reading, $options )
+
+Adds a new relationship of the type given in $options between the two readings,
+which may be specified by object or ID.  Returns a value of ( $status, @vectors)
+where $status is true on success, and @vectors is a list of relationship edges
+that were ultimately added.
+See L<Text::Tradition::Collation::Relationship> for the available options.
+
+=cut 
 
 sub BUILD {
     my $self = shift;
@@ -191,6 +317,15 @@ sub _stringify_args {
     return( $first, $second, $arg );
 }
 
+# Helper function for manipulating the graph.
+sub _objectify_args {
+       my( $self, $first, $second, $arg ) = @_;
+    $first = $self->reading( $first )
+        unless ref( $first ) eq 'Text::Tradition::Collation::Reading';
+    $second = $self->reading( $second )
+        unless ref( $second ) eq 'Text::Tradition::Collation::Reading';        
+    return( $first, $second, $arg );
+}
 ### Path logic
 
 sub add_path {
@@ -237,22 +372,40 @@ sub has_path {
        return $self->sequence->has_edge_attribute( $source, $target, $wit );
 }
 
-=head2 add_relationship( $reading1, $reading2, $definition )
+=head2 clear_witness( @sigil_list )
 
-Adds the specified relationship between the two readings.  A relationship
-is transitive (i.e. undirected); the options for its definition may be found
-in Text::Tradition::Collation::Relationship.
+Clear the given witnesses out of the collation entirely, removing references
+to them in paths, and removing readings that belong only to them.  Should only
+be called via $tradition->del_witness.
 
 =cut
 
-# Wouldn't it be lovely if edges could be objects, and all this type checking
-# and attribute management could be done via Moose?
+sub clear_witness {
+       my( $self, @sigils ) = @_;
+
+       # Clear the witness(es) out of the paths
+       foreach my $e ( $self->paths ) {
+               foreach my $sig ( @sigils ) {
+                       $self->del_path( $e, $sig );
+               }
+       }
+       
+       # Clear out the newly unused readings
+       foreach my $r ( $self->readings ) {
+               unless( $self->reading_witnesses( $r ) ) {
+                       $self->del_reading( $r );
+               }
+       }
+}
 
 sub add_relationship {
        my $self = shift;
     my( $source, $target, $opts ) = $self->_stringify_args( @_ );
-    return $self->relations->add_relationship( $source, $self->reading( $source ),
-       $target, $self->reading( $target ), $opts );
+    my( $ret, @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 $ret && $self->end->has_rank;
+    return( $ret, @vectors );
 }
 
 =head2 reading_witnesses( $reading )
@@ -276,13 +429,9 @@ sub reading_witnesses {
        return keys %all_witnesses;
 }
 
-=head2 Output method(s)
-
-=over
-
-=item B<as_svg>
+=head1 OUTPUT METHODS
 
-print $graph->as_svg( $recalculate );
+=head2 as_svg
 
 Returns an SVG string that represents the graph, via as_dot and graphviz.
 
@@ -304,33 +453,95 @@ sub as_svg {
     return $svg;
 }
 
-=item B<as_dot>
+=head2 svg_subgraph( $from, $to )
+
+Returns an SVG string that represents the portion of the graph given by the
+specified range.  The $from and $to variables refer to ranks within the graph.
+
+=cut
+
+sub svg_subgraph {
+    my( $self, $from, $to ) = @_;
+    
+    my $dot = $self->as_dot( $from, $to );
+    unless( $dot ) {
+       warn "Could not output a graph with range $from - $to";
+       return;
+    }
+    
+    my @cmd = qw/dot -Tsvg/;
+    my( $svg, $err );
+    my $dotfile = File::Temp->new();
+    ## TODO REMOVE
+    # $dotfile->unlink_on_destroy(0);
+    binmode $dotfile, ':utf8';
+    print $dotfile $dot;
+    push( @cmd, $dotfile->filename );
+    run( \@cmd, ">", binary(), \$svg );
+    $svg = decode_utf8( $svg );
+    return $svg;
+}
 
-print $graph->as_dot( $view, $recalculate );
+
+=head2 as_dot( $from, $to )
 
 Returns a string that is the collation graph expressed in dot
-(i.e. GraphViz) format.  The 'view' argument determines what kind of
-graph is produced.
-    * 'path': a graph of witness paths through the collation (DEFAULT)
-    * 'relationship': a graph of how collation readings relate to 
-      each other
+(i.e. GraphViz) format.  If $from or $to is passed, as_dot creates
+a subgraph rather than the entire graph.
 
 =cut
 
 sub as_dot {
-    my( $self, $view ) = @_;
-    $view = 'sequence' unless $view;
+    my( $self, $startrank, $endrank ) = @_;
+    
+    # Check the arguments
+    if( $startrank ) {
+       return if $endrank && $startrank > $endrank;
+       return if $startrank > $self->end->rank;
+       }
+       if( defined $endrank ) {
+               return if $endrank < 0;
+               $endrank = undef if $endrank == $self->end->rank;
+       }
+       
     # TODO consider making some of these things configurable
     my $graph_name = $self->tradition->name;
     $graph_name =~ s/[^\w\s]//g;
     $graph_name = join( '_', split( /\s+/, $graph_name ) );
+
+    my %graph_attrs = (
+       'rankdir' => 'LR',
+       'bgcolor' => 'none',
+       );
+    my %node_attrs = (
+       'fontsize' => 11,
+       'fillcolor' => 'white',
+       'style' => 'filled',
+       'shape' => 'ellipse'
+       );
+    my %edge_attrs = ( 
+       'arrowhead' => 'open',
+       'color' => '#000000',
+       'fontcolor' => '#000000',
+       );
+
     my $dot = sprintf( "digraph %s {\n", $graph_name );
-    $dot .= "\tedge [ arrowhead=open ];\n";
-    $dot .= "\tgraph [ rankdir=LR,bgcolor=none ];\n";
-    $dot .= sprintf( "\tnode [ fontsize=%d, fillcolor=%s, style=%s, shape=%s ];\n",
-                     11, "white", "filled", "ellipse" );
+    $dot .= "\tgraph " . _dot_attr_string( \%graph_attrs ) . ";\n";
+    $dot .= "\tnode " . _dot_attr_string( \%node_attrs ) . ";\n";
 
+       # Output substitute start/end readings if necessary
+       if( $startrank ) {
+               $dot .= "\t\"#SUBSTART#\" [ label=\"...\" ];\n";
+       }
+       if( $endrank ) {
+               $dot .= "\t\"#SUBEND#\" [ label=\"...\" ];\n";  
+       }
+       my %used;  # Keep track of the readings that actually appear in the graph
     foreach my $reading ( $self->readings ) {
+       # Only output readings within our rank range.
+       next if $startrank && $reading->rank < $startrank;
+       next if $endrank && $reading->rank > $endrank;
+        $used{$reading->id} = 1;
         # Need not output nodes without separate labels
         next if $reading->id eq $reading->text;
         my $label = $reading->text;
@@ -338,26 +549,59 @@ sub as_dot {
         $dot .= sprintf( "\t\"%s\" [ label=\"%s\" ];\n", $reading->id, $label );
     }
     
-    # TODO do something sensible for relationships
-
+       # Add the real edges
     my @edges = $self->paths;
+       my( %substart, %subend );
     foreach my $edge ( @edges ) {
-        my %variables = ( 'color' => '#000000',
-                          'fontcolor' => '#000000',
-                          'label' => join( ', ', $self->path_display_label( $edge ) ),
-            );
-        my $varopts = join( ', ', map { $_.'="'.$variables{$_}.'"' } sort keys %variables );
-        # Account for the rank gap if necessary
-        my $rankgap = $self->reading( $edge->[1] )->rank 
-               - $self->reading( $edge->[0] )->rank;
-               $varopts .= ", minlen=$rankgap" if $rankgap > 1;
-        $dot .= sprintf( "\t\"%s\" -> \"%s\" [ %s ];\n",
-                         $edge->[0], $edge->[1], $varopts );
+       # Do we need to output this edge?
+       if( $used{$edge->[0]} && $used{$edge->[1]} ) {;
+               my $label = $self->path_display_label( $self->path_witnesses( $edge ) );
+                       my $variables = { %edge_attrs, 'label' => $label };
+                       # Account for the rank gap if necessary
+                       if( $self->reading( $edge->[1] )->has_rank 
+                               && $self->reading( $edge->[0] )->has_rank
+                               && $self->reading( $edge->[1] )->rank 
+                               - $self->reading( $edge->[0] )->rank > 1 ) {
+                               $variables->{'minlen'} = $self->reading( $edge->[1] )->rank 
+                               - $self->reading( $edge->[0] )->rank;
+                       }
+                       my $varopts = _dot_attr_string( $variables );
+                       $dot .= sprintf( "\t\"%s\" -> \"%s\" %s;\n", 
+                               $edge->[0], $edge->[1], $varopts );
+        } elsif( $used{$edge->[0]} ) {
+               $subend{$edge->[0]} = 1;
+        } elsif( $used{$edge->[1]} ) {
+               $substart{$edge->[1]} = 1;
+        }
     }
+    # Add substitute start and end edges if necessary
+    foreach my $node ( keys %substart ) {
+       my $witstr = $self->path_display_label ( $self->reading_witnesses( $self->reading( $node ) ) );
+       my $variables = { %edge_attrs, 'label' => $witstr };
+        my $varopts = _dot_attr_string( $variables );
+        $dot .= "\t\"#SUBSTART#\" -> \"$node\" $varopts;";
+       }
+    foreach my $node ( keys %subend ) {
+       my $witstr = $self->path_display_label ( $self->reading_witnesses( $self->reading( $node ) ) );
+       my $variables = { %edge_attrs, 'label' => $witstr };
+        my $varopts = _dot_attr_string( $variables );
+        $dot .= "\t\"$node\" -> \"#SUBEND#\" $varopts;";
+       }
+       
     $dot .= "}\n";
     return $dot;
 }
 
+sub _dot_attr_string {
+       my( $hash ) = @_;
+       my @attrs;
+       foreach my $k ( sort keys %$hash ) {
+               my $v = $hash->{$k};
+               push( @attrs, $k.'="'.$v.'"' );
+       }
+       return( '[ ' . join( ', ', @attrs ) . ' ]' );
+}
+
 sub path_witnesses {
        my( $self, @edge ) = @_;
        # If edge is an arrayref, cope.
@@ -370,10 +614,10 @@ sub path_witnesses {
 }
 
 sub path_display_label {
-       my( $self, $edge ) = @_;
-       my @wits = $self->path_witnesses( $edge );
+       my( $self, @wits ) = @_;
        my $maj = scalar( $self->tradition->witnesses ) * 0.6;
        if( scalar @wits > $maj ) {
+               # TODO break out a.c. wits
                return 'majority';
        } else {
                return join( ', ', @wits );
@@ -381,14 +625,49 @@ sub path_display_label {
 }
                
 
-=item B<as_graphml>
+=head2 as_graphml
 
-print $graph->as_graphml( $recalculate )
+Returns a GraphML representation of the collation.  The GraphML will contain 
+two graphs. The first expresses the attributes of the readings and the witness 
+paths that link them; the second expresses the relationships that link the 
+readings.  This is the native transfer format for a tradition.
 
-Returns a GraphML representation of the collation graph, with
-transposition information and position information. Unless
-$recalculate is passed (and is a true value), the method will return a
-cached copy of the SVG after the first call to the method.
+=begin testing
+
+use Text::Tradition;
+
+my $READINGS = 311;
+my $PATHS = 361;
+
+my $datafile = 't/data/florilegium_tei_ps.xml';
+my $tradition = Text::Tradition->new( 'input' => 'TEI',
+                                      'name' => 'test0',
+                                      'file' => $datafile,
+                                      'linear' => 1 );
+
+ok( $tradition, "Got a tradition object" );
+is( scalar $tradition->witnesses, 13, "Found all witnesses" );
+ok( $tradition->collation, "Tradition has a collation" );
+
+my $c = $tradition->collation;
+is( scalar $c->readings, $READINGS, "Collation has all readings" );
+is( scalar $c->paths, $PATHS, "Collation has all paths" );
+is( scalar $c->relationships, 0, "Collation has all relationships" );
+
+# Add a few relationships
+$c->add_relationship( 'w123', 'w125', { 'type' => 'collated' } );
+$c->add_relationship( 'w193', 'w196', { 'type' => 'collated' } );
+$c->add_relationship( 'w257', 'w262', { 'type' => 'transposition' } );
+
+# Now write it to GraphML and parse it again.
+
+my $graphml = $c->as_graphml;
+my $st = Text::Tradition->new( 'input' => 'Self', 'string' => $graphml );
+is( scalar $st->collation->readings, $READINGS, "Reparsed collation has all readings" );
+is( scalar $st->collation->paths, $PATHS, "Reparsed collation has all paths" );
+is( scalar $st->collation->relationships, 3, "Reparsed collation has new relationships" );
+
+=end testing
 
 =cut
 
@@ -449,7 +728,7 @@ sub as_graphml {
        witness => 'string',                    # ID/label for a path
        relationship => 'string',               # ID/label for a relationship
        extra => 'boolean',                             # Path key
-       colocated => 'boolean',                 # Relationship key
+       scope => 'string',                              # Relationship key
        non_correctable => 'boolean',   # Relationship key
        non_independent => 'boolean',   # Relationship key
        );
@@ -524,7 +803,8 @@ sub as_graphml {
        }
        
        # Add the relationship graph to the XML
-       $self->relations->as_graphml( $root );
+       $self->relations->as_graphml( $graphml_ns, $root, \%node_hash, 
+               $node_data_keys{'id'}, \%edge_data_keys );
 
     # Save and return the thing
     my $result = decode_utf8( $graphml->toString(1) );
@@ -539,9 +819,7 @@ sub _add_graphml_data {
     $data_el->appendText( $value );
 }
 
-=item B<as_csv>
-
-print $graph->as_csv( $recalculate )
+=head2 as_csv
 
 Returns a CSV alignment table representation of the collation graph, one
 row per witness (or witness uncorrected.) 
@@ -566,23 +844,22 @@ sub as_csv {
     return join( "\n", @result );
 }
 
-=item B<make_alignment_table>
-
-my $table = $graph->make_alignment_table( $use_refs, \@wits_to_include )
+=head2 make_alignment_table( $use_refs, $include_witnesses )
 
 Return a reference to an alignment table, in a slightly enhanced CollateX
 format which looks like this:
 
  $table = { alignment => [ { witness => "SIGIL", 
-                             tokens => [ { t => "READINGTEXT" }, ... ] },
+                             tokens => [ { t => "TEXT" }, ... ] },
                            { witness => "SIG2", 
-                             tokens => [ { t => "READINGTEXT" }, ... ] },
+                             tokens => [ { t => "TEXT" }, ... ] },
                            ... ],
             length => TEXTLEN };
 
 If $use_refs is set to 1, the reading object is returned in the table 
 instead of READINGTEXT; if not, the text of the reading is returned.
-If $wits_to_include is set to a hashref, only the witnesses whose sigil
+
+If $include_witnesses is set to a hashref, only the witnesses whose sigil
 keys have a true hash value will be included.
 
 =cut
@@ -668,36 +945,20 @@ sub _turn_table {
     return $result;        
 }
 
-=back
-
-=head2 Navigation methods
-
-=over
-
-=item B<start>
-
-my $beginning = $collation->start();
-
-Returns the beginning of the collation, a meta-reading with label '#START#'.
-
-=item B<end>
-
-my $end = $collation->end();
+=head1 NAVIGATION METHODS
 
-Returns the end of the collation, a meta-reading with label '#END#'.
-
-
-=item B<reading_sequence>
-
-my @readings = $graph->reading_sequence( $first, $last, $path[, $alt_path] );
+=head2 reading_sequence( $first, $last, $sigil, $backup )
 
 Returns the ordered list of readings, starting with $first and ending
-with $last, along the given witness path.  If no path is specified,
-assume that the path is that of the base text (if any.)
+with $last, for the witness given in $sigil. If a $backup sigil is 
+specified (e.g. when walking a layered witness), it will be used wherever
+no $sigil path exists.  If there is a base text reading, that will be
+used wherever no path exists for $sigil or $backup.
 
 =cut
 
 # TODO Think about returning some lazy-eval iterator.
+# TODO Get rid of backup; we should know from what witness is whether we need it.
 
 sub reading_sequence {
     my( $self, $start, $end, $witness, $backup ) = @_;
@@ -730,9 +991,7 @@ sub reading_sequence {
     return @readings;
 }
 
-=item B<next_reading>
-
-my $next_reading = $graph->next_reading( $reading, $witpath );
+=head2 next_reading( $reading, $sigil );
 
 Returns the reading that follows the given reading along the given witness
 path.  
@@ -747,9 +1006,7 @@ sub next_reading {
     return $self->reading( $answer );
 }
 
-=item B<prior_reading>
-
-my $prior_reading = $graph->prior_reading( $reading, $witpath );
+=head2 prior_reading( $reading, $sigil )
 
 Returns the reading that precedes the given reading along the given witness
 path.  
@@ -773,8 +1030,8 @@ sub _find_linked_reading {
     # We have to find the linked path that contains all of the
     # witnesses supplied in $path.
     my( @path_wits, @alt_path_wits );
-    @path_wits = sort( $self->witnesses_of_label( $path ) ) if $path;
-    @alt_path_wits = sort( $self->witnesses_of_label( $alt_path ) ) if $alt_path;
+    @path_wits = sort( $self->_witnesses_of_label( $path ) ) if $path;
+    @alt_path_wits = sort( $self->_witnesses_of_label( $alt_path ) ) if $alt_path;
     my $base_le;
     my $alt_le;
     foreach my $le ( @linked_paths ) {
@@ -813,8 +1070,48 @@ sub _is_within {
     return $ret;
 }
 
+# Return the string that joins together a list of witnesses for
+# display on a single path.
+sub _witnesses_of_label {
+    my( $self, $label ) = @_;
+    my $regex = $self->wit_list_separator;
+    my @answer = split( /\Q$regex\E/, $label );
+    return @answer;
+}
+
+=head2 path_text( $sigil, $mainsigil [, $start, $end ] )
+
+Returns the text of a witness (plus its backup, if we are using a layer)
+as stored in the collation.  The text is returned as a string, where the
+individual readings are joined with spaces and the meta-readings (e.g.
+lacunae) are omitted.  Optional specification of $start and $end allows
+the generation of a subset of the witness text.
+
+=cut
+
+sub path_text {
+       my( $self, $wit, $backup, $start, $end ) = @_;
+       $start = $self->start unless $start;
+       $end = $self->end unless $end;
+       my @path = grep { !$_->is_meta } $self->reading_sequence( $start, $end, $wit, $backup );
+       return join( ' ', map { $_->text } @path );
+}
+
+=head1 INITIALIZATION METHODS
+
+These are mostly for use by parsers.
+
+=head2 make_witness_path( $witness )
+
+Link the array of readings contained in $witness->path (and in 
+$witness->uncorrected_path if it exists) into collation paths.
+Clear out the arrays when finished.
 
-## INITIALIZATION METHODS - for use by parsers
+=head2 make_witness_paths
+
+Call make_witness_path for all witnesses in the tradition.
+
+=cut
 
 # For use when a collation is constructed from a base text and an apparatus.
 # We have the sequences of readings and just need to add path edges.
@@ -850,6 +1147,13 @@ sub make_witness_path {
     $wit->clear_uncorrected_path;
 }
 
+=head2 calculate_ranks
+
+Calculate the reading ranks (that is, their aligned positions relative
+to each other) for the graph.  This can only be called on linear collations.
+
+=cut
+
 sub calculate_ranks {
     my $self = shift;
     # Walk a version of the graph where every node linked by a relationship 
@@ -882,7 +1186,7 @@ sub calculate_ranks {
         foreach my $n ( $self->sequence->successors( $r->id ) ) {
                my( $tfrom, $tto ) = ( $rel_containers{$r->id},
                        $rel_containers{$n} );
-               $DB::single = 1 unless $tfrom && $tto;
+               # $DB::single = 1 unless $tfrom && $tto;
             $topo_graph->add_edge( $tfrom, $tto );
         }
     }
@@ -900,7 +1204,6 @@ sub calculate_ranks {
         if( defined $node_ranks->{$rel_containers{$r->id}} ) {
             $r->rank( $node_ranks->{$rel_containers{$r->id}} );
         } else {
-            $DB::single = 1;
             die "No rank calculated for node " . $r->id 
                 . " - do you have a cycle in the graph?";
         }
@@ -942,8 +1245,13 @@ sub _assign_rank {
     return @next_nodes;
 }
 
-# Another method to make up for rough collation methods.  If the same reading
-# appears multiple times at the same rank, collapse the nodes.
+=head2 flatten_ranks
+
+A convenience method for parsing collation data.  Searches the graph for readings
+with the same text at the same rank, and merges any that are found.
+
+=cut
+
 sub flatten_ranks {
     my $self = shift;
     my %unique_rank_rdg;
@@ -952,7 +1260,7 @@ sub flatten_ranks {
         my $key = $rdg->rank . "||" . $rdg->text;
         if( exists $unique_rank_rdg{$key} ) {
             # Combine!
-            # print STDERR "Combining readings at same rank: $key\n";
+               # print STDERR "Combining readings at same rank: $key\n";
             $self->merge_readings( $unique_rank_rdg{$key}, $rdg );
         } else {
             $unique_rank_rdg{$key} = $rdg;
@@ -961,17 +1269,16 @@ sub flatten_ranks {
 }
 
 
-## Utility functions
-    
-# Return the string that joins together a list of witnesses for
-# display on a single path.
-sub witnesses_of_label {
-    my( $self, $label ) = @_;
-    my $regex = $self->wit_list_separator;
-    my @answer = split( /\Q$regex\E/, $label );
-    return @answer;
-}    
+=head1 UTILITY FUNCTIONS
+
+=head2 common_predecessor( $reading_a, $reading_b )
+
+Find the last reading that occurs in sequence before both the given readings.
 
+=head2 common_successor( $reading_a, $reading_b )
+
+Find the first reading that occurs in sequence after both the given readings.
+    
 =begin testing
 
 use Text::Tradition;
@@ -984,14 +1291,14 @@ my $t = Text::Tradition->new(
     );
 my $c = $t->collation;
 
-is( $c->common_predecessor( $c->reading('n9'), $c->reading('n23') )->id, 
+is( $c->common_predecessor( 'n9', 'n23' )->id, 
     'n20', "Found correct common predecessor" );
-is( $c->common_successor( $c->reading('n9'), $c->reading('n23') )->id, 
+is( $c->common_successor( 'n9', 'n23' )->id, 
     '#END#', "Found correct common successor" );
 
-is( $c->common_predecessor( $c->reading('n19'), $c->reading('n17') )->id, 
+is( $c->common_predecessor( 'n19', 'n17' )->id, 
     'n16', "Found correct common predecessor for readings on same path" );
-is( $c->common_successor( $c->reading('n21'), $c->reading('n26') )->id, 
+is( $c->common_successor( 'n21', 'n26' )->id, 
     '#END#', "Found correct common successor for readings on same path" );
 
 =end testing
@@ -1001,12 +1308,14 @@ is( $c->common_successor( $c->reading('n21'), $c->reading('n26') )->id,
 ## Return the closest reading that is a predecessor of both the given readings.
 sub common_predecessor {
        my $self = shift;
-       return $self->common_in_path( @_, 'predecessors' );
+       my( $r1, $r2 ) = $self->_objectify_args( @_ );
+       return $self->common_in_path( $r1, $r2, 'predecessors' );
 }
 
 sub common_successor {
        my $self = shift;
-       return $self->common_in_path( @_, 'successors' );
+       my( $r1, $r2 ) = $self->_objectify_args( @_ );
+       return $self->common_in_path( $r1, $r2, 'successors' );
 }
 
 sub common_in_path {
@@ -1041,6 +1350,6 @@ __PACKAGE__->meta->make_immutable;
 
 =over
 
-=item * Think about making Relationship objects again
+=item * Get rid of $backup in reading_sequence
 
 =back