Now works on both Python 2 and 3 #343

cclauss · 2017-11-24T14:33:02Z

Fixes #328, #265

Checklist

I have read the Contribution & Best practices Guide and my PR follows them.
My branch is up-to-date with the Upstream master branch.
I have added necessary documentation (if appropriate)

Changes proposed in this pull request:

Use absolute imports
import unquote() for both Python 2 and Python 3
Only encode on Python 2
Simplify by using flask.jsonify()
fix minor PEP8 and readability (Human readibility is low #265) issues

fossasia-bot · 2017-11-24T14:36:52Z

Hi @cclauss!

Looks like your PR has some conflicts. 😟
Could you resolve them and rebase on top of the latest upstream code ?

fossasia-bot · 2017-11-24T14:37:59Z

Hi @cclauss!

It looks like one or more of your builds have failed.
I've added the relevant info below to save you some time.

Build 662.2 has failed

fossasia-bot · 2017-11-24T14:39:53Z

Hi @cclauss!

Looks like your PR has some conflicts. 😟
Could you resolve them and rebase on top of the latest upstream code ?

fossasia-bot · 2017-11-24T14:40:50Z

Hi @cclauss!

It looks like one or more of your builds have failed.
I've added the relevant info below to save you some time.

Build 663.2 has failed

fossasia-bot · 2017-11-24T15:10:37Z

Hi @cclauss!

It looks like one or more of your builds have failed.
I've added the relevant info below to save you some time.

Build 663.2 has failed

fossasia-bot · 2017-11-24T15:35:29Z

Hi @cclauss!

Looks like your PR has some conflicts. 😟
Could you resolve them and rebase on top of the latest upstream code ?

fossasia-bot · 2017-11-27T18:26:01Z

Hi @cclauss!

Looks like your PR has some conflicts. 😟
Could you resolve them and rebase on top of the latest upstream code ?

fossasia-bot · 2017-11-30T10:57:50Z

Hi @cclauss!

Looks like your PR has some conflicts. 😟
Could you resolve them and rebase on top of the latest upstream code ?

cclauss · 2017-11-30T11:11:07Z

Rebased again. @ParthS007 @vaibhavsingh97 @AnshulMalik @bhaveshAn @gabru-md Your reviews please.

fossasia-bot · 2017-12-02T21:37:10Z

Hi @cclauss!

It looks like one or more of your builds have failed.
I've added the relevant info below to save you some time.

Build 698.2 has failed

bhaveshAn · 2017-12-02T23:56:33Z

@cclauss Checked your branch locally. Working fine. But Exalead search is not working with Python 3.

fossasia-bot · 2017-12-06T13:23:15Z

Hi @cclauss!

Looks like your PR has some conflicts. 😟
Could you resolve them and rebase on top of the latest upstream code ?

cclauss · 2017-12-06T23:44:51Z

Is it useful continually re-base my pull requests?

bhaveshAn · 2017-12-07T14:55:51Z

Resolve conflicts

fossasia-bot · 2017-12-07T22:46:44Z

Hi @cclauss!

Looks like your PR has some conflicts. 😟
Could you resolve them and rebase on top of the latest upstream code ?

fossasia-bot · 2017-12-07T22:47:34Z

Hi @cclauss!

It looks like one or more of your builds have failed.
I've added the relevant info below to save you some time.

Build 737.2 has failed

fossasia-bot · 2017-12-07T22:55:42Z

Hi @cclauss!

Looks like your PR has some conflicts. 😟
Could you resolve them and rebase on top of the latest upstream code ?

fossasia-bot · 2017-12-07T23:00:24Z

Hi @cclauss!

Looks like your PR has some conflicts. 😟
Could you resolve them and rebase on top of the latest upstream code ?

cclauss · 2017-12-07T23:00:54Z

Re-based yet again.

AnshulMalik · 2017-12-08T02:19:22Z

Hey @cclauss, I'll have a look sometime today. You have a lot of patience :)

AnshulMalik

Looks great!

AnshulMalik · 2017-12-08T09:34:04Z

app/scrapers/ask.py

@@ -25,7 +25,7 @@ def parseResponse(self, soup):
            title = div.div.a.text
            url = div.div.a['href']
            try:
-                p = div.find('p', {'class': 'PartialSearchResults-item-abstract'})
+                p = div.find('p', class_='PartialSearchResults-item-abstract')


Why this change?

PEP8 Compliance: This is equivalent functionality but requires fewer characters and PEP8 says that lines should 79 chars or less.

I find previous one more readable.
Let's wait for what others say? @bhaveshAn

The BeautifulSoup documentation tends to use the shorter form rather than creating and destroying a custom dict on every call. https://www.crummy.com/software/BeautifulSoup/bs4/doc/#searching-by-css-class class is of course a reserved word in Python, thus the trailing underscore convention.

Ok, but I still find the object notation more readable :P
That's why I was seeking other opinions @vaibhavsingh97

I removed the PEP8 fix. It is not worth the wait.

ParthS007

LGTM!

vaibhavsingh97

LGTM!! Will merge after codacy test result

cclauss · 2017-12-10T15:52:16Z

@vaibhavsingh97 #332

vaibhavsingh97 · 2017-12-10T15:58:15Z

Merging Since it got two approval

vaibhavsingh97 · 2017-12-10T15:58:59Z

@cclauss One last thing before merge, can you change the commit message to something relevant

cclauss · 2017-12-10T16:17:16Z

Commit message is now: Add support for Python 3

vaibhavsingh97 · 2017-12-10T16:38:51Z

Thanks for the patience and contribution

Lookup scrapers by name, not by initial (fossasia#327) Add support for Python 3 (fossasia#343) Fixes fossasia#357 Add Mojeek in README (fossasia#358) Lookup scrapers by name, not by initial (fossasia#327) Fixes fossasia#357 Add Mojeek in README (fossasia#358) Lookup scrapers by name, not by initial (fossasia#327) changesmade to server.py to prevent XML attacks defusedxml

fossasia-bot bot added the needs-review label Nov 24, 2017

fossasia-bot bot assigned cclauss Nov 24, 2017

cclauss force-pushed the Python-3-compatibility branch from 326927e to 60b5675 Compare November 24, 2017 14:39

cclauss force-pushed the Python-3-compatibility branch from 60b5675 to b7fb6c7 Compare November 24, 2017 15:35

cclauss force-pushed the Python-3-compatibility branch from b7fb6c7 to 770662d Compare November 27, 2017 18:25

cclauss force-pushed the Python-3-compatibility branch from 770662d to 84b3b4e Compare November 30, 2017 10:57

cclauss force-pushed the Python-3-compatibility branch from 84b3b4e to bff94b8 Compare December 2, 2017 21:35

cclauss force-pushed the Python-3-compatibility branch from bff94b8 to d037f7a Compare December 6, 2017 13:23

cclauss force-pushed the Python-3-compatibility branch from d037f7a to c59d194 Compare December 6, 2017 23:37

cclauss force-pushed the Python-3-compatibility branch from c59d194 to 3425fb5 Compare December 7, 2017 22:46

cclauss force-pushed the Python-3-compatibility branch from 301a7e5 to 7d9f6ff Compare December 7, 2017 23:00

cclauss mentioned this pull request Dec 8, 2017

Can we please shut down fossasia-bot? #376

Closed

AnshulMalik approved these changes Dec 8, 2017

View reviewed changes

cclauss mentioned this pull request Dec 8, 2017

Fix tests which incorporate network requests #368

Open

1 task

AnshulMalik approved these changes Dec 8, 2017

View reviewed changes

cclauss force-pushed the Python-3-compatibility branch 2 times, most recently from 219a29d to 8fc6ccd Compare December 8, 2017 12:43

ParthS007 reviewed Dec 8, 2017

View reviewed changes

cclauss force-pushed the Python-3-compatibility branch 5 times, most recently from 2b9e12a to ce6f989 Compare December 10, 2017 15:35

vaibhavsingh97 approved these changes Dec 10, 2017

View reviewed changes

Add support for Python 3

2f294ea

cclauss force-pushed the Python-3-compatibility branch from ce6f989 to 2f294ea Compare December 10, 2017 16:02

vaibhavsingh97 merged commit b04b781 into fossasia:master Dec 10, 2017

cclauss deleted the Python-3-compatibility branch December 10, 2017 17:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Now works on both Python 2 and 3 #343

Now works on both Python 2 and 3 #343

cclauss commented Nov 24, 2017

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 27, 2017

fossasia-bot bot commented Nov 30, 2017

cclauss commented Nov 30, 2017

fossasia-bot bot commented Dec 2, 2017

bhaveshAn commented Dec 2, 2017

fossasia-bot bot commented Dec 6, 2017

cclauss commented Dec 6, 2017

bhaveshAn commented Dec 7, 2017

fossasia-bot bot commented Dec 7, 2017

fossasia-bot bot commented Dec 7, 2017

fossasia-bot bot commented Dec 7, 2017

fossasia-bot bot commented Dec 7, 2017

cclauss commented Dec 7, 2017

AnshulMalik commented Dec 8, 2017 •

edited

Loading

AnshulMalik left a comment

AnshulMalik Dec 8, 2017

cclauss Dec 8, 2017

AnshulMalik Dec 8, 2017

cclauss Dec 8, 2017 •

edited

Loading

AnshulMalik Dec 8, 2017

cclauss Dec 8, 2017

ParthS007 left a comment

vaibhavsingh97 left a comment

cclauss commented Dec 10, 2017

vaibhavsingh97 commented Dec 10, 2017

vaibhavsingh97 commented Dec 10, 2017

cclauss commented Dec 10, 2017

vaibhavsingh97 commented Dec 10, 2017

Now works on both Python 2 and 3 #343

Now works on both Python 2 and 3 #343

Conversation

cclauss commented Nov 24, 2017

Checklist

Changes proposed in this pull request:

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 24, 2017

fossasia-bot bot commented Nov 27, 2017

fossasia-bot bot commented Nov 30, 2017

cclauss commented Nov 30, 2017

fossasia-bot bot commented Dec 2, 2017

bhaveshAn commented Dec 2, 2017

fossasia-bot bot commented Dec 6, 2017

cclauss commented Dec 6, 2017

bhaveshAn commented Dec 7, 2017

fossasia-bot bot commented Dec 7, 2017

fossasia-bot bot commented Dec 7, 2017

fossasia-bot bot commented Dec 7, 2017

fossasia-bot bot commented Dec 7, 2017

cclauss commented Dec 7, 2017

AnshulMalik commented Dec 8, 2017 • edited Loading

AnshulMalik left a comment

Choose a reason for hiding this comment

AnshulMalik Dec 8, 2017

Choose a reason for hiding this comment

cclauss Dec 8, 2017

Choose a reason for hiding this comment

AnshulMalik Dec 8, 2017

Choose a reason for hiding this comment

cclauss Dec 8, 2017 • edited Loading

Choose a reason for hiding this comment

AnshulMalik Dec 8, 2017

Choose a reason for hiding this comment

cclauss Dec 8, 2017

Choose a reason for hiding this comment

ParthS007 left a comment

Choose a reason for hiding this comment

vaibhavsingh97 left a comment

Choose a reason for hiding this comment

cclauss commented Dec 10, 2017

vaibhavsingh97 commented Dec 10, 2017

vaibhavsingh97 commented Dec 10, 2017

cclauss commented Dec 10, 2017

vaibhavsingh97 commented Dec 10, 2017

AnshulMalik commented Dec 8, 2017 •

edited

Loading

cclauss Dec 8, 2017 •

edited

Loading