Simple Web Crawler for Small Sites

Question

This script is my web crawlerfor small sites. Can you possibly review it for best coding practices?

Code

import bs4 as bs
import urllib.request
from urllib.parse import urlparse,urljoin
import pprint


class Page(object):
    def __init__(self,base_url,url):
        self.url = url
        self.base_url = base_url
    def soup(self):
        sauce = urllib.request.urlopen(self.url).read()
        return (bs.BeautifulSoup(sauce,'lxml'))
    def title(self):
        soup = self.soup()
        return soup.title.string
    def links(self):
        urls = []
        soup = self.soup()
        href = [i.get('href') for i in soup.findAll('a') ]
        links = [i for i in (list(map((lambda url : url if bool(urlparse(url).netloc) == True else urljoin (self.base_url, url)),href))) if i.startswith(self.base_url)]
        return links
    def map_page(self):
        map = {self.url:{'title':self.title(),'links':set(self.links())}}
        return map

def site_map(base_url):
    map_pages = {}
    links_to_map = [base_url]

    def check_and_add(url):
        if url not in map_pages:
            [links_to_map.append(i) for i in Page(base_url,url).links()]
            (map_pages.update(Page(base_url,url).map_page()))
            links_to_map.remove(url)
        else:
            links_to_map.remove(url)

    while links_to_map != []:
        url = links_to_map[0]
        check_and_add(url)

    pprint.pprint(map_pages)

Answer 1

I have made some changes to make it neater and more efficient. See the end of this response for the final solution. But I'll start with a few comments.

There is no point making your class inherit the object class, especially if you're working with Python 3.

class Page(object):

The soup() method seems to be called indiscriminately causing BeautifulSoup to parse the contents of the page in concern too many times (see the map_page() method, for example). It is probably more efficient to parse once, afterwhich you can save whatever (e.g. title and links)you need as object properties.

Using list and map in this context seems unnecessary. It helps if you strive for readability --- your future self will thank you.

links = [i for i in (list(map((lambda url : url if bool(urlparse(url).netloc) == True else urljoin (self.base_url, url)),href))) if i.startswith(self.base_url)]

Also, checking the truthiness of urlparse(url).netloc doesn't require you to coerce it to a boolean result first. Simply replacing if bool(urlparse(url).netloc) with if urlparse(url).netloc would suffice.

In the site_map() function, you can replace the while loop with something simpler, e.g.:

while links_to_map:
    check_and_add(links_to_map.pop())

I have also made other subjective simplifications that hopefully should be obvious to you.

import pprint
import urllib.request
from pprint import pprint
from urllib.parse import urlparse, urljoin

import bs4 as bs


class Page:
    def __init__(self, base_url, url):
        self.url = url
        self.base_url = base_url
        self.souped = None
        self.title = None
        self.links = None

    def soup(self):
        def clean(url):
            return url if urlparse(url).netloc else urljoin(self.base_url, url)

    sauce = urllib.request.urlopen(self.url).read()
    self.souped = bs.BeautifulSoup(sauce, "lxml")
    self.title = self.souped.title.string
    hrefs = set([clean(i.get("href")) for i in self.souped.findAll("a")])
    self.links = [link for link in hrefs if link.startswith(self.base_url)]
    return self

    @property
    def map_page(self):
        lookup = {self.url: {"title": self.title, "links": self.links}}
        return lookup


def site_map(base_url):
    map_pages = {}
    links_to_map = [base_url]

    def check_and_add(url):
    if url not in map_pages:
        page = Page(base_url, url).soup()
        links_to_map.extend(page.links)
        map_pages.update(page.map_page)

    while links_to_map:
        check_and_add(links_to_map.pop())

    pprint(map_pages)

Answer 2

not exhaustive but some feedback. There are some big "No No"s in this code as well as some minor formatting things.

First the big "No No"s.

1) Do not use list comprehension unless you plan do use the result: [links_to_map.append(i) for i in Page(base_url,url).links()] Do this in a normal for loop.

2) DO NOT change a list you are iterating over (i.e. links_to_map in your lest method).

Other stuff:
return statements don't need ():

return (bs.BeautifulSoup(sauce,'lxml'))

Should just be:
return bs.BeautifulSoup(sauce,'lxml')

Avoid needless () as well:

(map_pages.update(Page(base_url,url).map_page())

to:
map_pages.update(Page(base_url,url).map_page()

Empty lists are considered False so:

while links_to_map != []:

can just be:
while links_to_map:

My last tip. Consider using a set for links_to_map instead of a list since the removing step will be significantly faster.