Ilya_Torch_itorch2001@gmail.com by IlyaTorch · Pull Request #8 · epam-python-courses-7-bsu/FinalTaskRssParser

IlyaTorch · 2019-11-05T17:19:30Z

Iteration 1
Options --version, --json, --limit, --verbose are ready. Add option --verbose. Split classes into files. Add description of project to file README.md.

Options --version, --json, --limit are ready. Split classes into files. Iteration 1 Options --version, --json, --limit are ready. Split classes into files.

Add option cl verbose

Add description of project to file README.md

HenadziStantchik · 2019-11-05T19:01:57Z

Please do NOT close this and open new pull request anymore, Our comments do not save and it is hard to track your progress

HenadziStantchik · 2019-11-05T19:20:36Z

Here is some feedback about your application work:

If you to check version without specifying RSS url, you wil get:
rss_reader.py: error: the following arguments are required: source
It should be that you do not need any RSS url for checking app version.
You got some problems with encoding. Check image below:

If limit is not specified you will get only 1 piece of news right now, while it should give you the whole available feed
Sometimes you do not get image link in "Links" (see the image below)

P.S. I used --limit 999 option to test last threee points.

IlyaTorch · 2019-11-05T21:03:02Z

1), 3) -fixed
2) Is it necessary?
4) Img tag exists on this page and it has an attribute "alt" while it doesn't have an attribute "src". What do I need to do in this case?

Remove redundants strings and correct method parse_html in the file Entry.py, Remove redundants empty lines in all the files. Make a decorator for logging and move to other file Logging. Combine parsing command line args into function and move it to other file Console_parse. Fixe the error when check version without RSS url. And fixe output: print whole available feed instead of an article.

HenadziStantchik · 2019-11-05T21:52:02Z

Is it necessary?

Yes

Img tag exists on this page and it has an attribute "alt" while it doesn't have an attribute "src". What do I need to do in this case?

That is certainly strange. Did you visit the url? if there is no src then there should be no image available on the page.

P.S. In the future use labels. Check slack for more info.

Remove unsed improts in files Entry.py, rss_reader.py

HenadziStantchik · 2019-11-06T21:49:08Z

Also please do not request review using site interface, because it triggers a bit different functionality and compells us to create feedbacks abouth your PR and either merge it or close it. Cancel review request please

Edit output of links in file Handler.py; Edit fiel README.md; Add files ConsoleParse.py and Logging.py; Add parsing special symbols and ASCII codes to function parse_html (file Entry.py)

IlyaTorch · 2019-11-06T22:58:11Z

When can I cancel review request?

Remove condition extra condition from file Handler.py and add argument limit check; Remove adding img-links without attr src

HenadziStantchik · 2019-11-07T11:06:24Z

When launching your application in this way:
python rss_reader.py https://news.yahoo,com/rss/
I get an exception:

This bug is inconsistent because this appears not every application run.

IlyaTorch · 2019-11-07T20:10:51Z

It's strange because when I launch application using:
python rss_reader.py https://news.yahoo,com/rss/
command I get only my expect-exception.

HenadziStantchik · 2019-11-23T13:15:13Z

I have no problems with encoding in all my browsers. I rewrote write_entries_to_html methon but I'm not sure that it'll help. What can I do with it?

Then this means that the problem on my side. It is ok then.

HenadziStantchik · 2019-11-23T13:19:33Z

I did not discover any more issues with iteration 4

Add tests fo class Entry; Edit output methods and correct writing of date for Entry objects; Edit some Handler class methods using context managers for working with files and checking if file exists with os.path.exists() and using isinstance() function istead of type(*) == ?; Simplify if-else structure in rss_reader.py file

Edit converting to html and pdf functions for case of several images on the page

Add some tests

IlyaTorch · 2019-11-23T19:55:49Z

Please look at my tests. Can you explain me why, when I add new tests, % of coverage doesn't change?

IlyaTorch · 2019-11-23T19:59:02Z

And how do the files for which I didn't write a single test show 100% coverage? I check coverage so
coverage run rss_reader.py
coverage report -m or coverage html

IlyaTorch · 2019-11-23T19:59:46Z

I wrote tests only for files Entry.py and Hanler.py

HenadziStantchik · 2019-11-24T11:43:03Z

Coverage for exceptions is 100% because they do not contain any statements that need to be tested, only class declarations.

Coverage for logging is 100% because you decorated almost every function with logging decorator, so it implicitly called in each your test, so the coverage thinks that you tested it.

If you wrote tests only for entry and handler and there is some percentage on other modules, that means that the functions you wrote tests for call some functions from other modules and this makes coverage think that you tested these functions too.

Try to check some documentation on coverage, I am sure you will find solution to your problem

Add more tests for Entry and Handler classes

Move tests to the rss_reader folder; Add prints when html and pdf documents were created

Move tests to rss_reader folder

Add dockstrings;

Change memu options; Delete extra exceptions in Handler

HenadziStantchik · 2019-12-03T13:59:27Z

+5 mains files of project:
+* rss_reader.py - the file which runs the application
+* ConsoleParse.py - contains code which parses arguments from console
+* Entry.py - contains class Entry which represent an article
+* Handler.py - contains class Handler which performes functions of processing objects Entry
+* Logging.py - contains decorator for printing loggs in stdout


It would be nice if you kept it up to date.

HenadziStantchik · 2019-12-03T14:21:55Z

Here is feedback on your application:
Iteration 1:
--json does not work with Cyrillic symbols.
Iteration 2:
Installs fine, all iterations work.
Iteration 3:
User should be able specify date in YYYYMMDD:

Iteration 4:
Everything works fine.

Commit messages are very informative and correspond to the recommended commit message style.
Code is well structured and readable, but there are some large methods which lower readability a bit. The code corresponds to the PEP8 guidelines.
Tests are decent, but some of them are failing test_write_entries_to_html (and without). keep in mind that tests should not depend on any external factors such as OS or etc. Coverage percentage is high enough.

Add work --json with Cyrillic symbols. Correct date format in YYYYMMDD

Edit saving of the news cache

Add search by url in date option

Add colorize option

…f data hiding

IlyaTorch added 3 commits November 5, 2019 16:24

Iteration 1

22b4987

Options --version, --json, --limit are ready. Split classes into files. Iteration 1 Options --version, --json, --limit are ready. Split classes into files.

Iteration 1

dd8f4a2

Add option cl verbose

Iteration 1

e00637a

Add description of project to file README.md