Skip to content

Conversation

@nvaulin
Copy link
Member

@nvaulin nvaulin commented Feb 26, 2024

Review UBC

Copy link

@ailiskab-hub ailiskab-hub left a comment

Choose a reason for hiding this comment

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

Хорошая работа, все выполнено четко)

min_gc_bound = 0
max_gc_bound = gc_bounds
else:
return print("Incorrect gc_bounds input")

Choose a reason for hiding this comment

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

Круто, что учли момент с вводом не числа

Copy link
Member Author

Choose a reason for hiding this comment

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

Но return print кажется это что-то странное... Что он вернёт?

elif isinstance(index, (list, tuple)) and len(index) == 2:
return self.seq[index[0]:index[1]]

def __str__(self):

Choose a reason for hiding this comment

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

Мне кажется, неплохо было бы использовать repr

Copy link
Member Author

Choose a reason for hiding this comment

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

Обрати внимане - у тебя маркдаун распознал __ как обозначение жирного текста. Чтобы было как ты хотела - надо ставить штрихи с двух сторон (они на букве ё) - так будет отображено как код

return gc_fraction(self.seq)


class DNASequence(NucleicAcidSequence):

Choose a reason for hiding this comment

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

было бы неплохо провести проверку на то что последовательность является ДНК

Copy link

@greenbergM greenbergM left a comment

Choose a reason for hiding this comment

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

Работа аккуратная, код понятный!) Но все таки, мне кажется, с наследованием у аминокислотных последовательностей можно было реализовать больше

max_len_bound = length_bounds
else:
return print("Incorrect length_bounds input")

Choose a reason for hiding this comment

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

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

max_len_bound = length_bounds[1]
elif type(length_bounds) is tuple and len(length_bounds) == 0:
min_len_bound = 0
max_len_bound = 2**32

Choose a reason for hiding this comment

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

В шапке функции уже же есть значения по умолчанию...

seq_quality = sum(record.letter_annotations["phred_quality"]) / len(record)

if (min_len_bound <= seq_length <= max_len_bound) and (min_gc_bound <= seq_gc_content <= max_gc_bound) and (seq_quality >= quality_threshold):
filtered_dict_key.append(record)

Choose a reason for hiding this comment

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

Гуд!

with open(output_filename, 'w') as output_handle:
SeqIO.write(filtered_dict_key, output_handle, 'fastq')

print(f"Filtered FastQ sequences saved to '{output_filename}'")

Choose a reason for hiding this comment

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

Мелочь, а приятно, что выводит это)

return str(self.seq)

def check_alphabet(self):
return set(str(self.seq)) <= set("ACGTUacgtu")

Choose a reason for hiding this comment

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

Не учитывает аминокислотные последовательности, хотя они от этого класса наследуются!

else:
using_dict = DNA_COMPLEMENT
for nucl in self.seq:
complement_seq.append(using_dict[nucl])

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.

4 participants