Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Week2 #15

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Week2 #15

wants to merge 18 commits into from

Conversation

Abusagit
Copy link
Member

No description provided.

Copy link
Member

@ad3002 ad3002 left a comment

Choose a reason for hiding this comment

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

Все круто! Местами код слишком сложный, можно упростить.
Со сложным кодом две проблемы 1) если ты в команде, то все должны понимать что код делает 2) ты должен быть уверен что если в команде только ты, что ты через пару лет будешь понимать, что ты написал

@@ -3,6 +3,9 @@ __pycache__/
*.py[cod]
*$py.class

# WebStorm
.idea/
Copy link
Member

Choose a reason for hiding this comment

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

Удали папку .idea из репозитория, но оставь у себя. Пойми как это сделать.

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 names
pivot = names[random.randint(0, len(names) - 1)]

middle = [i for i in names if i == pivot]
Copy link
Member

Choose a reason for hiding this comment

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

ну вот точно хочется переменную name, а не i, тут же строки

Copy link
Member Author

Choose a reason for hiding this comment

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

Согласен, но она же безликая по идее)

a = list(set(a1) | set(a2))

for bypass in range(1, len(a)):
for k in range(len(a) - bypass):
Copy link
Member

Choose a reason for hiding this comment

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

почему k? глобальная идея в том, что переменные которые одинаковые по сути называются одинаково у всех, код должен читаться без комментариев исходя из названий переменных

Copy link
Member

Choose a reason for hiding this comment

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

тут как раз i бы

Copy link
Member Author

Choose a reason for hiding this comment

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

Ну, да, логично
Взял свой старый код сюда

nonlocal transition
if number:
string = "{} {}"
hundreds = {0: '',
Copy link
Member

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.

Это не противоречит пеп8 кэмэл-стайлу?

```python
# merge sort practise
def merge(a1, a2):
sorted_arr = [None for _ in range(len(a1) + len(a2))]
Copy link
Member

Choose a reason for hiding this comment

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

использование None вместо 0 здесь оправдано если у тебя где то будет проверка x is None

```python
def create_phone_number(n):
string = "({}{}{}) {}{}{}-{}{}{}{}"
return string.format(*n)
Copy link
Member

Choose a reason for hiding this comment

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

Класс!

file = input()

with open(fr"D:\Downloads\{file}.txt") as f_read, open("new.fasta", 'w') as f_write:
for line in f_read:
Copy link
Member

Choose a reason for hiding this comment

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

лучше while, в цикле for не ожидается что будет меняться состояние f_read, например для словарей бы ошибку выкинуло, тут мало кода и видно где, а если бы этот ридлайн был бы где-то в глубинах?

threshold = int(f_read.readline())

a = f_read.readlines()
qualities = [None for _ in range(len(a) // 4)]
Copy link
Member

Choose a reason for hiding this comment

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

не надо None



def get_incorrect_phred(file):
with open(rf"D:\Downloads\{file}.txt") as f_read:
Copy link
Member

Choose a reason for hiding this comment

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

почему rf?

with open(rf"D:\Downloads\{file}.txt") as f_read, open(rf"filtered{file}.fastaq", 'w') as f_write:
cut_off = int(f_read.readline())
lines = iter(f_read.readlines())
while lines:
Copy link
Member

Choose a reason for hiding this comment

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

мне нравится

line = fh.readline()
while line:
     ...
     line = fh.readline()

Должно работать!

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