Как я могу изменить это на «идиоматический» Perl? - PullRequest
8 голосов
/ 23 октября 2009

Я начинаю углубляться в Perl, но у меня возникают проблемы с написанием кода "Perl-ly" вместо написания C на Perl. Как я могу изменить следующий код, чтобы использовать больше идиом Perl, и как мне учиться идиомам?

Просто объяснение того, что он делает: эта процедура является частью модуля, который выравнивает последовательности ДНК или аминокислот (используя Needelman-Wunch, если вы заботитесь о таких вещах). Он создает два двумерных массива, один для хранения оценки для каждой позиции в двух последовательностях, а другой для отслеживания пути, чтобы выравнивание с наивысшей оценкой можно было воссоздать позже. Это работает нормально, но я знаю, что делаю вещи не очень кратко и четко.

edit: Это было для назначения. Я закончил, но хочу немного почистить мой код. Подробности реализации алгоритма можно найти на сайте класса , если кто-то из вас заинтересован.

sub create_matrix {
    my $self = shift;
    #empty array reference
    my $matrix = $self->{score_matrix};
    #empty array ref
    my $path_matrix = $self->{path_matrix};
    #$seq1 and $seq2 are strings set previously
    my $num_of_rows = length($self->{seq1}) + 1;
    my $num_of_columns = length($self->{seq2}) + 1;

    #create the 2d array of scores
    for (my $i = 0; $i < $num_of_rows; $i++) {
        push(@$matrix, []);
        push(@$path_matrix, []);
        $$matrix[$i][0] = $i * $self->{gap_cost};
        $$path_matrix[$i][0] = 1;
    }

    #fill out the first row
    for (my $i = 0; $i < $num_of_columns; $i++) {
        $$matrix[0][$i] = $i * $self->{gap_cost};
        $$path_matrix[0][$i] = -1;
    }
    #flag to signal end of traceback
    $$path_matrix[0][0] = 2;
    #double for loop to fill out each row
    for (my $row = 1; $row < $num_of_rows; $row++) {
        for (my $column = 1; $column < $num_of_columns; $column++) {
            my $seq1_gap = $$matrix[$row-1][$column] + $self->{gap_cost};
            my $seq2_gap = $$matrix[$row][$column-1] + $self->{gap_cost};
            my $match_mismatch = $$matrix[$row-1][$column-1] + $self->get_match_score(substr($self->{seq1}, $row-1, 1), substr($self->{seq2}, $column-1, 1));
            $$matrix[$row][$column] = max($seq1_gap, $seq2_gap, $match_mismatch);

            #set the path matrix
            #if it was a gap in seq1, -1, if was a (mis)match 0 if was a gap in seq2 1
            if ($$matrix[$row][$column] == $seq1_gap) {
                $$path_matrix[$row][$column] = -1;
            }
            elsif ($$matrix[$row][$column] == $match_mismatch) {
                $$path_matrix[$row][$column] = 0;
            }
            elsif ($$matrix[$row][$column] == $seq2_gap) {
                $$path_matrix[$row][$column] = 1;
            }
        }
    }
}

Ответы [ 6 ]

9 голосов
/ 23 октября 2009

Вы получаете несколько предложений относительно синтаксиса, но я бы также предложил более модульный подход, если по какой-либо другой причине эта читаемость кода. Гораздо проще освоить код, если вы можете увидеть общую картину, прежде чем беспокоиться о деталях низкого уровня.

Ваш основной метод может выглядеть следующим образом.

sub create_matrix {
    my $self = shift;
    $self->create_2d_array_of_scores;
    $self->fill_out_first_row;
    $self->fill_out_other_rows;
}

И у вас также есть несколько небольших методов, таких как:

n_of_rows
n_of_cols
create_2d_array_of_scores
fill_out_first_row
fill_out_other_rows

И вы можете пойти еще дальше, определив еще более мелкие методы - геттеры, сеттеры и так далее. На этом этапе ваши методы среднего уровня, такие как create_2d_array_of_scores, не будут напрямую касаться базовой структуры данных.

sub matrix      { shift->{score_matrix} }
sub gap_cost    { shift->{gap_cost}     }

sub set_matrix_value {
    my ($self, $r, $c, $val) = @_;
    $self->matrix->[$r][$c] = $val;
}

# Etc.
8 голосов
/ 23 октября 2009

Одно простое изменение заключается в использовании for таких циклов:

for my $i (0 .. $num_of_rows){
    # Do stuff.
}

Для получения дополнительной информации см. Документацию по Perl foreach loops и оператор диапазона .

7 голосов
/ 23 октября 2009

Вместо разыменования ваших двумерных массивов, таких как:

$$path_matrix[0][0] = 2;

сделать это:

$path_matrix->[0][0] = 2;

Кроме того, вы делаете много операторов if / then / else для сопоставления с конкретными подпоследовательностями: это может быть лучше записано в виде given операторов (эквивалент perl5.10 C * switch) Читайте об этом на perldoc perlsyn :

given ($matrix->[$row][$column])
{
    when ($seq1_gap)       { $path_matrix->[$row][$column] = -1; }
    when ($match_mismatch) { $path_matrix->[$row][$column] = 0; }
    when ($seq2_gap)       { $path_matrix->[$row][$column] = 1; }
}
7 голосов
/ 23 октября 2009

У меня есть и другие комментарии, но вот первое замечание:

my $num_of_rows = length($self->{seq1}) + 1;
my $num_of_columns = length($self->{seq2}) + 1;

Итак, $self->{seq1} и $self->{seq2} - строки, и вы продолжаете обращаться к отдельным элементам, используя substr Я бы предпочел хранить их как массивы символов:

$self->{seq1} = [ split //, $seq1 ];

Вот как бы я написал это:

sub create_matrix {
    my $self = shift;

    my $matrix      = $self->{score_matrix};
    my $path_matrix = $self->{path_matrix};

    my $rows = @{ $self->{seq1} };
    my $cols = @{ $self->{seq2} };

    for my $row (0 .. $rows) {
        $matrix->[$row]->[0] =  $row * $self->{gap_cost};
        $path_matrix->[$row]->[0] = 1;
    }

    my $gap_cost = $self->{gap_cost};

    $matrix->[0] = [ map { $_ * $gap_cost } 0 .. $cols ];
    $path_matrix->[0] = [ (-1) x ($cols + 1) ];

    $path_matrix->[0]->[0] = 2;

    for my $row (1 .. $rows) {
        for my $col (1 .. $cols) {
            my $gap1 = $matrix->[$row - 1]->[$col] + $gap_cost;
            my $gap2 = $matrix->[$row]->[$col - 1] + $gap_cost;
            my $match_mismatch =
                $matrix->[$row - 1]->[$col - 1] +
                $self->get_match_score(
                    $self->{seq1}->[$row - 1],
                    $self->{seq2}->[$col - 1]
                );

            my $max = $matrix->[$row]->[$col] =
                max($gap1, $gap2, $match_mismatch);

            $path_matrix->[$row]->[$col] = $max == $gap1
                    ? -1
                    : $max == $gap2
                    ? 1
                    : 0;
            }
        }
    }
5 голосов
/ 23 октября 2009

Большая часть вашего кода манипулирует 2D-массивами. Я думаю, что самым большим улучшением было бы переключение на использование PDL , если вы хотите много делать с массивами, особенно если речь идет об эффективности. Это модуль Perl, который обеспечивает отличную поддержку массивов. Базовые подпрограммы реализованы на языке C для повышения эффективности, поэтому они тоже быстрые.

0 голосов
/ 25 октября 2009

Я бы посоветовал взглянуть на CPAN для предыдущих решений или примеров того, как что-то делать в Perl. Вы смотрели на Алгоритм :: NeedlemanWunsch ?

Документация к этому модулю включает пример для сопоставления последовательностей ДНК. Вот пример использования матрицы подобия из wikipedia .

#!/usr/bin/perl -w
use strict;
use warnings;
use Inline::Files;                 #multiple virtual files inside code
use Algorithm::NeedlemanWunsch;    # refer CPAN - good style guide

# Read DNA sequences
my @a = read_DNA_seq("DNA_SEQ_A");
my @b = read_DNA_seq("DNA_SEQ_B");

# Read Similarity Matrix (held as a Hash of Hashes)
my %SM = read_Sim_Matrix();

# Define scoring based on "Similarity Matrix" %SM
sub score_sub {
    if ( !@_ ) {
        return -3;                 # gap penalty same as wikipedia)
    }
    return $SM{ $_[0] }{ $_[1] };    # Similarity Value matrix
}

my $matcher = Algorithm::NeedlemanWunsch->new( \&score_sub, -3 );
my $score = $matcher->align( \@a, \@b, { align => \&check_align, } );

print "\nThe maximum score is $score\n";

sub check_align {
    my ( $i, $j ) = @_;              # @a[i], @b[j]
    print "seqA pos: $i, seqB pos: $j\t base \'$a[$i]\'\n";
}

sub read_DNA_seq {
    my $source = shift;
    my @data;
    while (<$source>) {
        push @data, /[ACGT-]{1}/g;
    }
    return @data;
}

sub read_Sim_Matrix {

    #Read DNA similarity matrix (scores per Wikipedia)
    my ( @AoA, %HoH );
    while (<SIMILARITY_MATRIX>) {
        push @AoA, [/(\S+)+/g];
    }

    for ( my $row = 1 ; $row < 5 ; $row++ ) {
        for ( my $col = 1 ; $col < 5 ; $col++ ) {
            $HoH{ $AoA[0][$col] }{ $AoA[$row][0] } = $AoA[$row][$col];
        }
    }
    return %HoH;
}

__DNA_SEQ_A__
A T G T A G T G T A T A G T
A C A T G C A
__DNA_SEQ_B__
A T G T A G T A C A T G C A
__SIMILARITY_MATRIX__
-  A  G  C  T
A  10  -1  -3  -4
G  -1  7  -5  -3
C  -3  -5  9  0
T  -4  -3  0  8

А вот пример вывода:

seqA pos: 7, seqB pos: 2  base 'G'
seqA pos: 6, seqB pos: 1  base 'T'
seqA pos: 4, seqB pos: 0  base 'A'

The maximum score is 100
Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...