Skip to content

Shymansky Pavel #23

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

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

pavl1n
Copy link

@pavl1n pavl1n commented Nov 9, 2019

Description: There is a problem with the output of information in printArticles() function. Also few issues connected with argparse module, unrealised json format fully. And russian is not supported here at moment.

Description: There is a problem with the output of information in printArticles() function. Also few issues connected with argparse module, unrealised json format fully. And russian is not supported here at moment.
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<module type="PYTHON_MODULE" version="4">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Как я уже писал в чате:

Мета-файлы вашего текстового редактора в репозитории не должны хранится. Если вы используете PyCharm, то вам стоит добавить .idea/ в .gitignore файл.

@AlexeiBuzuma AlexeiBuzuma added the [Deadline] Iteration 1-2 This is a marker for first and second iterations deadline. label Nov 17, 2019
pavl1n and others added 2 commits November 21, 2019 16:57
In this version was added such feautures as unittest, some logs, saving news in 'json' format, and fixed bugs related with output
setup.py Outdated
description = 'CLI utility to process RSS',
author = 'Pavel Shymansky',
py_module = ['rss.parser','arg.py'],
install_requires = ['feedparser', 'bs4'], #!!!!!!!!!!!!!!!!!!!не забытЬ!!!!!!!!!!!!!
Copy link
Collaborator

Choose a reason for hiding this comment

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

Веселый коментарий :)
Обычно для подобного рода комментариев для "незабывания" используют " # ToDo: "
многие среды разработки умеют красиво их подчеркивать

rss_parser.py Outdated
logger.info('Limit is {}'.format(args.limit))


def versionInfo(args):
Copy link
Collaborator

Choose a reason for hiding this comment

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

название метода должно быть маленькими буквами через нижнее подчеркивание

loggerfile.py Outdated
import logging
extra = {'app_name':'rss_parser'}

logger = logging.getLogger(__name__)
Copy link
Collaborator

Choose a reason for hiding this comment

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

в глобальной области видимости обычно не выполняют никакой логики. Лучше перенести эту логику в отдельную функцию, а потом ее вызвать из какой-нибудь функции (например main, или что-то в этом роде)

arg.py Outdated
@@ -0,0 +1,10 @@
import argparse

parser = argparse.ArgumentParser(description='Pure python command-line RSS reader')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Такая же ситуация, нужно перенести эту логику в отдельную функцию

setup.py Outdated
python_requires='>=3.8',
entry_points ='''
[console_scripts]
rss-parser=rss_parser: main
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. в тех задании описано, что утилита должна называться rss-reader
  2. не работает после установки:
# rss-parser --help
Traceback (most recent call last):
  File "/usr/local/bin/rss-parser", line 5, in <module>
    from rss_parser import main
ModuleNotFoundError: No module named 'rss_parser'

Also was added feature --date which cache news, but now it not fully implemented. Method --version was moved in separate file. Logs was added to the another functions
Also was added feature --date which cache news, but now it not fully implemented. Method --version was moved in separate file. Logs was added to the another functions
Add some more logs, add to_pdf function but not implemented fully, add requirements.txt, README.md
@pavl1n pavl1n changed the title [WIP] Shymansky Pavel Shymansky Pavel Dec 1, 2019
parser.add_argument("--verbose", action='store_true', help='Outputs verbose status messages')
parser.add_argument("--limit", help='Limit news topics', type=int)
parser.add_argument("--date", help='Shows cached news on introduced day', type=int)
parser.add_argument("--to_pdf",action='store_true', help = 'Converts news into PDF format' )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Конвертация сделана только в один формат?

@AlexeiBuzuma
Copy link
Collaborator

К сожалению, не работает из-за проблем с импортом

# rss-reader --help
Traceback (most recent call last):
  File "/usr/local/bin/rss-reader", line 5, in <module>
    from rss_reader import main
ModuleNotFoundError: No module named 'rss_reader'

@AlexeiBuzuma AlexeiBuzuma added 1-2 iteration | not implemented and removed Hard Deadline [Deadline] Iteration 1-2 This is a marker for first and second iterations deadline. labels Dec 11, 2019
@pavl1n
Copy link
Author

pavl1n commented Dec 14, 2019

@AlexeiBuzuma а можешь пожалуйста подсказать как вы проверяли работы? Потому что я чекал на чистой винде и голом линуксе и всё работало

@AlexeiBuzuma
Copy link
Collaborator

Работы проверялись в докер контейнерах python:latest.
В чистом контейнере клонировался репозиторий с кодом и производилась установка через pip install .
более подробно этот процесс описывался в нашем чате в слаке

Пункты, которые могут быть улучшены:

  1. файл date_converter.py. Стандартная библиотека пайтона содержит в себе много готовых вещей. Например есть модуль datetime, который возволит сделать то, что реализованно в функции convert_date
  2. не все имена переменных соответствуют pep8
  3. Нет использования ООП, хотя эта парадигма очень уместна для данной задачи
  4. функционально для verbose может быть реализована с помощью функциональности из модуля logging
  5. функция parse вызывается в приложении 8 раз, каждый раз при вызове этой функции отправляется запрос на получения XML с новостями. Это достаточно не эффективно, плюс во время работы приложения новости могут измениться и на последний запрос, например, данный скрипт может работать уже с отличными данными от первоначальных новостей.

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

Successfully merging this pull request may close these issues.

2 participants