Skip to content
This repository was archived by the owner on Oct 5, 2022. It is now read-only.

requested changes from commit after issue #5#8

Open
loopyd wants to merge 6 commits into
rushilsrivastava:masterfrom
loopyd:master
Open

requested changes from commit after issue #5#8
loopyd wants to merge 6 commits into
rushilsrivastava:masterfrom
loopyd:master

Conversation

@loopyd
Copy link
Copy Markdown

@loopyd loopyd commented Mar 10, 2018

from issue #5:

  • includes the hash argument as an option with affecting the url argument's functionality (you can still use the program as before, but now with the addition of --hash yes if you wish
  • includes the error counter you voiced you liked, as a property of download_image
  • slight output cleanup - errors where being printed twice in your version (unnecessary), exception is now passed directly once to error() and printed optimally
  • no extra config file needed, this one directly uses properties of download_image

includes the hash argument as an option with affecting the url argument's functionality
includes the error counter
slight output cleanup - errors where being printed twice (unessecary), exception is now passed directly once to error() and printed optimally
@loopyd
Copy link
Copy Markdown
Author

loopyd commented Mar 11, 2018

FYI, your change to use io.open instead of open produces the same error i was intending to fix in python 2.7 with my structural workarounds. be advised you will have to patch this. i left it intact to show you why this occours. (check travis)

The correct method for 2.7, is:

        with io.open('dataset/logs/google/source.html', 'w', encoding='utf-8') as f:

The python 3 syntax works and should be left as it is.

Sorry there where two here, forgot the encoding specifier.

@rushilsrivastava
Copy link
Copy Markdown
Owner

rushilsrivastava commented Mar 11, 2018

Will start the review when build checks with Travis CI.

@rushilsrivastava
Copy link
Copy Markdown
Owner

rushilsrivastava commented Mar 11, 2018

Opening the file with utf-8 encoding should be enough, correct? Could you give me an example to test with?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants