Skip to content

Conversation

@grishchenkoira
Copy link
Owner

No description provided.

grishchenkoira and others added 30 commits October 7, 2023 23:46
Copy link

@IvanKozlov98 IvanKozlov98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Фильтратор: 3/5 (на тестовом примере не сработал)
Биол. последовательности: 14/19

Штраф за отсутствие pr после дедлайна: -2

Таким образом, 15 баллов за эту часть.


write_filtered_sequences_to_fastq(filtered_seq, unfiltered_seq, new_file_name)

return ('Sequences are filtered!')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не соответствует типу в декларации функции

if quality_threshold > 40:
raise ValueError(f'Wrong quality threshold!')

records = list(SeqIO.parse(input_path, "fastq"))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это будет работать только с небольшими fastq-файлами.
Представьте, что вам попался файл-метагенома и нужно сделать фильтрацию.
Если переписывать все данные в питоновский объект -- программа лопнет :)
В данном случае это просто не нужно: читаете очередную запись, узнаем ок/(не ок), если ок - записываем её в выходной файл и приступаем к следующей записи.

filtered_seq = {record_id: str(record.seq) for record_id, record in filtered_by_quality.items()}
unfiltered_seq = {record.id: str(record.seq) for record in records if record.id not in filtered_seq}

if filtered_file_name == None:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Принято писать if filtered_file_name is None

Comment on lines +151 to +155
filtered_by_gc = analyse_gc(records, min_gc, max_gc)

filtered_by_length = filter_by_length(filtered_by_gc.values(), min_length, max_length)

filtered_by_quality = filter_by_quality(filtered_by_length.values(), quality_threshold)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

неудачный подбор имён: filtered_by_quality -- на самом деле filtered по 3 признакам, а не по одному

Comment on lines +90 to +95
for key, value in unfiltered_sequences.items():
records_unfiltered = []
sequence = Seq(value)
record = SeqRecord(sequence, id=key, description="")
records_unfiltered.append(record)
SeqIO.write(records_unfiltered, output_handle, "fasta")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

повторяющейся код
Не бойтесь писать небольшие функции внутри функций, чтобы избегать такого

Comment on lines +214 to +215
self.dna_alphabet = set('AaTtGgCc')
self.rna_alphabet = set('AaUuGgCc')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Классы должны быть независимы друг от друга.
Зачем NucleicAcidSequnce алфавит ДНК или РНК.
NucleicAcidSequnce может иметь свой собственный алфавит (какой?)

Представьте, что вы захотите использовать NucleicAcidSequnce вообще в другой задаче -- зачем там алфавиты ДНК или РНК ?)

Comment on lines +242 to +243
if (set(self.seq).issubset(self.dna_alphabet) and isinstance(self, DNASequence)) or (set(self.seq).issubset(self.rna_alphabet) and isinstance(self, RNASequence)):
return True

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Именно так в задании написано не делать :)

При вызове dna.complement() или условного dna.check_alphabet() должны будут вызываться соответствующие методы из NucleicAcidSequence. Иначе говоря - полиморфизм.

Если заметить, то is_valid_alphabet и complement имеют схожую реализацию. Разница состоит только в разных значениях complement_dict для каждого класса. То есть предполагается некоторое общее, абстрактное решение в базовом классе, которое будет опираться на конкретные, определенные в классах поля (или методы).

Небольшой пример :)
вы определили абстрактный метод foo в базовом классе A следующим образом.

  class A:
        ...
        def foo(self):
              return self.get_t() + self.calc_b()

тогда, если объект класса-наследника B, наследует этот метод, то чтобы он работал как нужно - следует определить методы get_t и calc_b

  class B(A):
        ...
        def get_t(self):
            # specific implementation for 'B' objects

        def calc_b(self):
            # specific implementation for 'B' objects

При этом объекты другого класса-наследника C могут иметь другую реализацию методов get_t и calc_b .

   class C(A):
        ...
        def get_t(self):
            # specific implementation for 'C' objects

        def calc_b(self):
            # specific implementation for 'C' objects

В итоге, объекты B и C имеют одинаковые по структуре, но немного разные по смыслу метод foo (поскольку опираются на разные конкретные методы get_t и calc_b)

Чтобы почувствовать полиморфизм, постарайтесь доделать это задание :)

:rtype: str
:return: complement sequence
"""
if self.is_alphabet_correct():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

чтобы избавиться от ненужной вложености обычно делают так

     if not self.is_alphabet_correct():
          return
     # other code...

:return: complement sequence
"""
if self.is_alphabet_correct():
complement_seq = str()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

= ""

Comment on lines +259 to +260
if self.seq[i] in self.complement_dict:
complement_seq += (self.complement_dict[self[i]])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а если условие неверно?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants