Oleg_Slavashevich_oslavashevish@gmail.com by OlegSlavashevich · Pull Request #15 · epam-python-courses-7-bsu/FinalTaskRssParser

OlegSlavashevich · 2019-11-09T08:58:00Z

No description provided.

dzhigailo · 2019-11-09T10:10:08Z

@@ -0,0 +1,20 @@
+import argparse
+
+parser = argparse.ArgumentParser(description='Getting info from sites')


Entire logic of this module should placed in a function.

dzhigailo · 2019-11-09T10:10:46Z

@@ -0,0 +1,61 @@
+import feedparser
+from pprint import pprint
+from myargparse import *


Please, do not use import * in your solutions

dzhigailo · 2019-11-09T12:34:42Z

+import json
+
+
+def parse(url):


I'm not sure that you have to add this function. All it does - return result of another function.

HenadziStantchik · 2019-11-09T16:40:15Z

Please check your code for PEP8 compliance. You can use such tools as pycodestyle (but make maximum line length to 120 when you check)

HenadziStantchik · 2019-11-09T16:49:29Z

Here is some feedback on your application work:

There should not be any HTML-code in the printed feed.
Your application has encoding problem:

If user did not specify the limit, he/she should get all available feed, not just one entry.

Please do not forget that you have to add --version and --verbose arguments in Iteration 1.

dzhigailo · 2019-11-14T19:33:33Z

@@ -0,0 +1,22 @@
+INFO:root:Website is working


Please, remove your local .log file from repository

dzhigailo · 2019-11-14T19:35:33Z

@@ -0,0 +1,15 @@
+import logging
+from clean_output import clean_title


This is an unused import

dzhigailo · 2019-11-14T19:36:30Z

+
+
+def logg(article):
+    logging.debug("Title: " + clear_title(article['title']))


This code is not working, because there is no clear_title function

HenadziStantchik · 2019-11-15T11:26:59Z

+import re
+
+def clean_title(text):
+    "Delete unnecessary symbols"


Here and below: please make your docstrings a bit more specific

HenadziStantchik · 2019-11-15T11:42:53Z

--verbose does not work: I get a NameError exception with traceback to stdout.
it is impossible to use --version without specifying RSS url

HenadziStantchik · 2019-11-15T11:43:38Z

Please fix your commit messages as some of them do not correspond to commit message guidelines.

HenadziStantchik · 2019-11-15T11:44:34Z

Please thoroughly test your app before pushing your commits to the remote repository.

HenadziStantchik · 2019-12-03T15:10:32Z

+    parser.add_argument(
+        '--limit',
+        type=int, 
+        help='Limit news topics if this parameter provided'
+    )


Here and below: it is ok to make that into one line. It will be readable anyway.

HenadziStantchik · 2019-12-03T15:11:04Z

+    month = {'Jan': '1',
+             'Feb': '2',
+             'Mar': '3',
+             'Apr': '4',
+             'May': '5',
+             'Jun': '6',
+             'Jul': '7',
+             'Aug': '8',
+             'Sep': '9',
+             'Oct': '10',
+             'Nov': '11',
+             'Dec': '12'}
+    day = date[5:7]
+    month_int = month[date[8:11]]
+    year = date[12:16]


It is better to use datetime lib for this.

HenadziStantchik · 2019-12-03T15:11:29Z

@@ -0,0 +1,107 @@
+import feedparser
+#from arg import parsargs, VERSION


please avoid commented-out code in pushed commits.

HenadziStantchik · 2019-12-03T15:11:55Z

+    for i in articles:
+        var = True
+        for j in news_csv:
+            if i['published'] != j[4]:


It is better to avoid one-character variable names.

HenadziStantchik · 2019-12-03T15:18:33Z

+    with open('news.csv', 'a', newline='') as csvfile:
+        fieldnames = ['link','title', 'img', 'summary', 'published']
+
+        writer = csv.DictWriter(csvfile, fieldnames=fieldnames)
+
+        writer.writerow(articles)


As far as I understand if you will get news 5 times from the same rss source, then you will have these news 5 times repeated in your file

HenadziStantchik · 2019-12-03T15:20:17Z

+            addcsv(i)
+
+def main():
+    console_args = arg.parsargs()


If you import specific function using from then you should just use it as is: parsargs, without arg

Right now this will break your application.

HenadziStantchik · 2019-12-03T15:23:18Z

+    a = []
+    with open('news.csv', 'r') as csvFile:
+        reader = csv.reader(csvFile)
+        for row in reader:
+            a.append(row)


If there is no such file (and the first time you launch program on new machine there won't be such file) this line will crash your application with FileNotFoundError

HenadziStantchik · 2019-12-03T15:23:27Z

Here is feedback on your app:
Unfortunatelly your app does not work:

There are no tests.
The code is mostly readable, but does not correspond to the PEP8 guidelines. You may want to use pycodestyle tool in the future.
Commit messages could be more informative, however they mostly correspond to the recommended commit message style.

OlegSlavashevich added 3 commits November 9, 2019 11:26

Gets source information

7a10e17

Gets entries information

60df8e8

Add argparse

8e1078b

dzhigailo reviewed Nov 9, 2019

View reviewed changes

Added argparse logic to function and

88d7ac5

HenadziStantchik reviewed Nov 9, 2019

View reviewed changes

Comment thread final_task/rss_reader/rss_reader.py Outdated

HenadziStantchik reviewed Nov 9, 2019

View reviewed changes

Comment thread final_task/rss_reader/rss_reader.py Outdated

HenadziStantchik reviewed Nov 9, 2019

View reviewed changes

Comment thread final_task/rss_reader/rss_reader.py Outdated

OlegSlavashevich added 3 commits November 12, 2019 15:50

ignore this commit)

a5a583d

delete myargparse.py

d2d342e

Add --verbose and --version

22f7706

OlegSlavashevich changed the title ~~Oleg_Slavashevich_oslavashevish@gmail.com~~ Oleg_Slavashevich_oslavashevish@gmail.com - PLEASE REVIEW Nov 14, 2019

OlegSlavashevich added 3 commits November 14, 2019 19:19

Add setup.py

c0095cc

Fixed problems with the Title

822b2eb

super small change

eb54633

dzhigailo reviewed Nov 14, 2019

View reviewed changes

dzhigailo changed the title ~~Oleg_Slavashevich_oslavashevish@gmail.com - PLEASE REVIEW~~ Oleg_Slavashevich_oslavashevish@gmail.com Nov 14, 2019

HenadziStantchik reviewed Nov 15, 2019

View reviewed changes

Comment thread final_task/rss_reader/arg.py Outdated

HenadziStantchik reviewed Nov 15, 2019

View reviewed changes

Comment thread final_task/rss_reader/clean_output.py Outdated

HenadziStantchik reviewed Nov 15, 2019

View reviewed changes

Comment thread final_task/rss_reader/rss_reader.py Outdated

HenadziStantchik reviewed Nov 15, 2019

View reviewed changes

Comment thread final_task/setup.py

HenadziStantchik reviewed Nov 15, 2019

View reviewed changes

Comment thread final_task/rss_reader/rss_reader.py Outdated

Iteration 3

eb04e6b

dzhigailo added the CODE FREEZE label Nov 25, 2019

HenadziStantchik reviewed Dec 3, 2019

View reviewed changes

		@@ -0,0 +1,20 @@
		import argparse

		parser = argparse.ArgumentParser(description='Getting info from sites')

		@@ -0,0 +1,15 @@
		import logging
		from clean_output import clean_title



		def logg(article):
		logging.debug("Title: " + clear_title(article['title']))

		@@ -0,0 +1,107 @@
		import feedparser
		#from arg import parsargs, VERSION

Conversation

OlegSlavashevich commented Nov 9, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HenadziStantchik commented Nov 9, 2019

Uh oh!

Uh oh!

Uh oh!

HenadziStantchik commented Nov 9, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HenadziStantchik commented Nov 15, 2019

Uh oh!

HenadziStantchik commented Nov 15, 2019

Uh oh!

HenadziStantchik commented Nov 15, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HenadziStantchik Dec 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HenadziStantchik commented Dec 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HenadziStantchik Dec 3, 2019 •

edited

Loading