Alex Birillo #50
Alex Birillo #50
Conversation
AlexeiBuzuma
added
the
MissedPRDeadline
rss_reader/arg.py
Outdated
| @@ -0,0 +1,9 @@ | |||
| import argparse | |||
|
|
|||
| parser = argparse.ArgumentParser(description = "simple rss-reader") | |||
There was a problem hiding this comment.
в глобальной секции не должно содержаться никакой логики. советую перенести парсинг аргументов в отдельную функцию
rss_reader/news.py
Outdated
| import re | ||
| import html | ||
| import json | ||
| from arg import * |
There was a problem hiding this comment.
Очень не рекомендуется использовать импорт через *
rss_reader/news.py
Outdated
| if not args.limit: | ||
| prepared_news.append(newsdict) | ||
| with open("data.json", "w") as output: | ||
| json.dump(prepared_news, output, indent = 3, ensure_ascii = False) |
There was a problem hiding this comment.
Разный размер табуляции
rss_reader/rss_reader.py
Outdated
| if args.version: | ||
| print("Current version: " + version.version) | ||
|
|
||
| if (args.verbose): |
There was a problem hiding this comment.
советую посмотреть в сторону модуля logging
Almost all of comments have been fixed. Logs are added through logging. Logic moved to functions.
Merge branch 'generalTask' of https://github.com/biralex-bsuir/PythonHomework into generalTask
AlexeiBuzuma
added
the
[Deadline] Iteration 1-2
Implemented caching and search by date
add to README.md description related to --date
| with open("README.md", "r") as f: | ||
| long_description = f.read() | ||
|
|
||
| setuptools.setup( |
There was a problem hiding this comment.
при запуске в докер контейнере python:latest падает вот с такой ошибкой
# rss_reader https://news.tut.by/rss/geonews/brest.rss
Traceback (most recent call last):
File "/usr/local/bin/rss_reader", line 8, in <module>
sys.exit(main())
File "/usr/local/lib/python3.8/site-packages/rss_reader/rss_reader.py", line 24, in main
news.caching_news(captured_news)
File "/usr/local/lib/python3.8/site-packages/rss_reader/news.py", line 57, in caching_news
owner_name = os.getlogin()
OSError: [Errno 6] No such device or addressUpdated README.md to the current state
ghost
changed the title
AlexeiBuzuma
added
[1-2 iteration] Minimal requirements checked
[3-4 iteration] Minimal requirements checked
and removed
Hard Deadline
[Deadline] Iteration 1-2
| except EOFError: | ||
| break | ||
|
|
||
| def compare_dates(newsdate): |
There was a problem hiding this comment.
Док стринги очень полезная вещь. Есть много функций, которые не содержат документации совсем :(
| news_date = datetime.strptime(newsdate[5:len(newsdate)-13], "%d %b %Y") | ||
| news_date = str(news_date)[:10] | ||
| requared_date = arg.args.date | ||
| requared_date = requared_date[0:4] + "-" + requared_date[4:6] + "-" + requared_date[6:8] |
There was a problem hiding this comment.
очень много магических чисел. очень сложно понять, почему они тут есть и что они значат
| return news_date == requared_date | ||
|
|
||
| def find_news_in_cache(): | ||
| isNewsFind = False |
There was a problem hiding this comment.
Не весь код соответствует pep8
| def encode_image(path): | ||
| """Gets the path to the image and returns it into encoded string""" | ||
| with open(path, "rb") as im: | ||
| encoded_image = base64.b64encode(im.read()) |
There was a problem hiding this comment.
Немного съехала табуляция
| else: | ||
| file_path = """C:\\news.rss""" | ||
|
|
||
| with open(file_path, "rb") as input: |
There was a problem hiding this comment.
имя input существует изначально в build-in (функция, которая позволяет считать ввод с клавиатуры)
лучше не переопределяь уже существующие имена, это может иногда сильно запутывать
| from rss_reader.rss_fb2 import encode_image | ||
| from rss_reader import arg | ||
|
|
||
| class handlers(unittest.TestCase): |
There was a problem hiding this comment.
Слабое покрытие юнит-тестами
| img_file.write(img.content) | ||
| img_file.close() | ||
|
|
||
| def remove_image(i): |
There was a problem hiding this comment.
что такое? image_name или image_index?
лучше не создавать имена в один символ, такой код тяжело читается
| @@ -0,0 +1,20 @@ | |||
|
|
|||
There was a problem hiding this comment.
Коммит сообщения должны нести информацию о том, что сделано внутри этого коммита. Например сообщение "[WIP] Alex Birillo" в целом не несет в себе никакой информацию.
| from colored import fore, style | ||
|
|
||
|
|
||
| class News(): |
There was a problem hiding this comment.
Скобки можно опустить
| def write_to_json(news): | ||
| """Creates data.json document in the directory which the application was launched""" | ||
| prepared_news = [] | ||
| for i in range(len(news)): |
There was a problem hiding this comment.
for index, item in enumerate(news):
I am very stupid and apologize for that. I will try to correct all errors you indicated