DjangoBB LoFi version

Полная версия: Прокомментируйте код новичка

Начало » Python для новичков » Прокомментируйте код новичка

1 2

jcrow

Окт. 31, 2011 08:51:26

Задача:
Проанализировать страницу и подсчитать количество внутренних и внешних ссылок. Ну и самы ссылки получить в виде удобном для дальнейшей работы.
Код:

#!/usr/bin/env python
# -*- coding: utf-8 -*-
import urllib
import urlparse
import lxml.html
import chardet
import argparse
import sys

args_parser = argparse.ArgumentParser(description='Parses url and calculates external and internal links')
args_parser.add_argument('url', metavar='URL', type=str, help='Provide an url')
args = args_parser.parse_args()


class Url():
    pass

class Parser():
    def __init__(self, url):
        self.url = url
        self.domain = urlparse.urlparse(url).netloc
        html = urllib.urlopen(url).read()
        self.html_encoding = chardet.detect(html)['encoding']
        self.html = lxml.html.document_fromstring(html)


    def parse(self):
        links_external = []
        links_internal = []

        for item in self.html.cssselect('a'):
            if item.get('href') != None:
                item_url = urllib.unquote(item.get('href'))
                item_domain = urlparse.urlparse(item_url).netloc
                
                urlo = Url()
                
                if item.text and item.text.strip():
                    name = item.text
                else:
                    name = u'Нет имени'
                
                urlo.name = name
                urlo.url = item_url
                
                if not (item_domain == '' or item_domain.endswith(self.domain)):
                    links_external.append(urlo)
                else:
                    links_internal.append(urlo)

        return {'links_internal': links_internal, 'links_external': links_external}



parser = Parser(args.url)
result = parser.parse()



print u'Всего ссылок: %s, из них внешних: %s' % (len(result['links_external']) + len(result['links_internal']), len(result['links_external']))
print '-----------------------'
for item in result['links_internal'][:5]:
    if type(item.url).__name__ == 'str':
        item.url = item.url.decode(parser.html_encoding)
    if type(item.name).__name__ == 'str':
        item.name = item.name.decode(parser.html_encoding)
    print u"%s %s" % (item.name, item.url)
print '-----------------------'
for item in result['links_external'][:5]:
    if type(item.url).__name__ == 'str':
        item.url = item.url.decode(parser.html_encoding)
    if type(item.name).__name__ == 'str':
        item.name = item.name.decode(parser.html_encoding)
    print "%s %s" % (item.name.encode('utf-8'), item.url.encode('utf-8'))
print '-----------------------'

Что вы скажете за этот код? В какую сторону стараться?

Окт. 31, 2011 10:16:10

В какую сторону стараться?

В эту: http://www.python.org/dev/peps/pep-0008/
эту: http://www.artima.com/weblogs/viewpost.jsp?thread=4829
и эту: http://www.python.org/dev/peps/pep-0020/
А потом
в эту: http://pypi.python.org/pypi/pylint
и в эту: http://pychecker.sourceforge.net/
и в эту: http://pypi.python.org/pypi/pep8

А потом результат опять сюда.

n2b

Окт. 31, 2011 12:07:58

Ed, а под винду есть такие вещи?

jcrow

Окт. 31, 2011 12:18:20

Вот новая версия:

#!/usr/bin/env python
# -*- coding: utf-8 -*-
'''
Parser program
'''
import urllib
import urlparse
import lxml.html
import chardet
import argparse


class Url():
    '''
    Stores data about url
    '''
    def __init__(self):
        pass


class Parser():
    '''
    Parses url and get links
    '''
    def __init__(self, url):
        self.url = url
        self.domain = urlparse.urlparse(url).netloc
        html = urllib.urlopen(url).read()
        self.html_encoding = chardet.detect(html)['encoding']
        self.html = lxml.html.document_fromstring(html)

    def parse(self):
        '''
        Parser urls
        '''
        links_external = []
        links_internal = []
        for item in self.html.cssselect('a'):
            if item.get('href') != None:
                item_url = urllib.unquote(item.get('href'))
                item_domain = urlparse.urlparse(item_url).netloc

                urlo = Url()

                if item.text and item.text.strip():
                    name = item.text
                else:
                    name = u'Нет имени'

                urlo.name = name
                urlo.url = item_url

                if not (item_domain == '' or
                        item_domain.endswith(self.domain)
                       ):
                    links_external.append(urlo)
                else:
                    links_internal.append(urlo)
        return {'links_internal': links_internal,
                'links_external': links_external}


def main():
    '''
    Main func
    '''
    args_parser = argparse.ArgumentParser(
        description='Parses url and calculates external and internal links')
    args_parser.add_argument('url', metavar='URL',
                             type=str, help='Provide an url'
                            )
    args = args_parser.parse_args()

    parser = Parser(args.url)
    result = parser.parse()

    print u'Всего ссылок: %s, из них внешних: %s' % (
                len(result['links_external']) + len(result['links_internal']),
                len(result['links_external'])
            )
    print '-----------------------'
    for item in result['links_internal'][:5]:
        if isinstance(item.url, str):
            item.url = item.url.decode(parser.html_encoding)
        if isinstance(item.name, str):
            item.name = item.name.decode(parser.html_encoding)
        print u"%s %s" % (item.name, item.url)
    print '-----------------------'
    for item in result['links_external'][:5]:
        if isinstance(item.url, str):
            item.url = item.url.decode(parser.html_encoding)
        if isinstance(item.name, str):
            item.name = item.name.decode(parser.html_encoding)
        print "%s %s" % (item.name.encode('utf-8'), item.url.encode('utf-8'))
    print '-----------------------'


if __name__ == "__main__":
    main()

Изменения коснулись (в основном) стиля: укоротил строки, добавил докстринги, использую isinstance() вместо странного.
Поставил pep8, pylint и pychecker.
pep8 не жалуется,
pychecker говорит:

$ pychecker parser.py 
Processing module parser (parser.py)...
  ImportError: No module named lxml.html

Пока не разобрался как его завести.
pylint говорит “Your code has been rated at 7.54/10” и вот на что жалуется:

W: 51,16:Parser.parse: Attribute 'url' defined outside __init__
W: 50,16:Parser.parse: Attribute 'name' defined outside __init__
R: 13,0:Url: Too few public methods (0/2)
E: 27,22:Parser.__init__: Instance of 'ParseResult' has no 'netloc' member
E: 41,30:Parser.parse: Instance of 'ParseResult' has no 'netloc' member
R: 21,0:Parser: Too few public methods (1/2)

Too few public method мне понятно, но что ж поделать – такие у меня классы. А остальное я не понял.

Андрей Светлов

Окт. 31, 2011 18:06:02

n2b, оно же все вроде бы под виндой работает.

Окт. 31, 2011 20:50:01

jcrow
Вот новая версия:
Изменения коснулись (в основном) стиля: укоротил строки, добавил докстринги, использую isinstance() вместо странного.
Поставил pep8, pylint и pychecker.
pep8 не жалуется,
pychecker говорит:
$ pychecker parser.py 
Processing module parser (parser.py)...
  ImportError: No module named lxml.html

Это типа
from lxml import html мб нужно было?

Окт. 31, 2011 21:32:01

Ну, это уже можно смотреть по крайней мере.

Это
W: 51,16:Parser.parse: Attribute ‘url’ defined outside __init__
W: 50,16:Parser.parse: Attribute ‘name’ defined outside __init__
Из-за странного использования класса Url. Проинициализируйте аттрибуты в __init__ - перестанет. Но вообще если этот Url нужен только для хранения имени и урла, то я бы советовал использовать dict., поскольку класс этот только усложняет понимание кода.

E: 27,22:Parser.__init__: Instance of ‘ParseResult’ has no ‘netloc’ member
Тут pylint подглюкивает, не может понять, что в ParseResult есть атрибут ‘netloc’. Там атрибуты сделаны через namedtuple, поэтому видимо. Забейте, все нормально.

Что-то по-моему не так с перекодированием: У меня при выводе по-русски только то, что в коде выводится. А названия линков кракозябрами. впрочем может это у меня что-то глючит, не знаю.

Использование isinstance - плохой стиль. Если у вас имена и урлы либо в виде строк либо в виде уникода, то что-то тут не так. Нужно разбираться.

if item.text and item.text.strip(): - избыточно

if item.get('href') != None: - не достаточно ли просто if item.get(“href”) ?

              if not (item_domain == '' or
                        item_domain.endswith(self.domain)
                       ):
                    links_external.append(urlo)
                else:
                    links_internal.append(urlo)

эквивалентно

 
              if item_domain == '' or item_domain.endswith(self.domain):
                    links_internal.append(urlo)
              else:
                    links_external.append(urlo)

Но второе читается проще.

Длинные названия типа links_external, html_encoding
Название атрибута item_url - непонятно.

Для того, чтобы найти все линки можно попробовать использоваться xpath, а не перебирать все элементы ‘a’. Вроде xpath в lxml есть.

Пока достаточно. Исправите - приходите еще :)

PS: мой pychecker 0.8.19-3 молчит.

jcrow

Ноя. 1, 2011 15:09:13

Исправленная версия:

#!/usr/bin/env python
# -*- coding: utf-8 -*-
'''
Parser program
'''
import urllib
import urlparse
import lxml.html
import chardet
import argparse


class Parser():
    '''
    Parses url and get links
    '''
    def __init__(self, url):
        self.url = url
        self.domain = urlparse.urlparse(url).netloc
        html = urllib.urlopen(url).read()
        self.encoding = chardet.detect(html)['encoding']
        self.html = lxml.html.document_fromstring(html)

    def parse(self):
        '''
        Parser urls
        '''
        links_external = []
        links_internal = []
        for item in self.html.cssselect('a'):
            if item.get('href'):
                url = urllib.unquote(item.get('href'))
                domain = urlparse.urlparse(url).netloc
                if item.text and item.text.strip():
                    name = item.text
                else:
                    name = u'Нет имени'
                if domain == '' or domain.endswith(self.domain):
                    links_internal.append({'name': name, 'url': url})
                else:
                    links_external.append({'name': name, 'url': url})
        return {'links_internal': links_internal,
                'links_external': links_external}


def main():
    '''
    Main func
    '''
    args_parser = argparse.ArgumentParser(
        description='Parses url and calculates external and internal links')
    args_parser.add_argument('url', metavar='URL',
                             type=str, help='Provide an url'
                            )
    args = args_parser.parse_args()

    parser = Parser(args.url)
    result = parser.parse()

    print u'Всего ссылок: %s, из них внешних: %s' % (
                len(result['links_external']) + len(result['links_internal']),
                len(result['links_external'])
            )
    print '-----------------------'
    for item in result['links_internal'][:5]:
        if isinstance(item['url'], str):
            item['url'] = item['url'].decode(parser.encoding)
        if isinstance(item['name'], str):
            item['name'] = item['name'].decode(parser.encoding)
        print u"%s | %s" % (item['name'], item['url'])
    print '-----------------------'
    for item in result['links_external'][:5]:
        if isinstance(item['url'], str):
            item['url'] = item['url'].decode(parser.encoding)
        if isinstance(item['name'], str):
            item['name'] = item['name'].decode(parser.encoding)
        print u"%s | %s" % (item['name'], item['url'])
    print '-----------------------'

if __name__ == "__main__":
    main()

Ed
Что-то по-моему не так с перекодированием

Вот именно! Название и ссылка иногда приходит как str, иногда как unicode. Я пока не разобрался даже как сформулировать вопрос на эту тему, но чувствую, что мои пляски с перекодированием избыточны.

Ed
if item.text and item.text.strip(): - избыточно

Мне тоже не нравится, но item.text мне отдает lxml'ем. Кроме нормальных случаев, он может быть None, или str c текстом ‘\n’. Оба раза мне надо заменить это текстом ‘Нет имени’. А strip() на объекте None ругается. Потому такой код. Не знаю как можн улучшить.

Больше спасибо за ответы. В них (внезапно!) много пользы.

s0rg

Ноя. 1, 2011 16:21:09

isinstance(item['url'], str)

Лучше заменить на

isinstance(item['url'], basestring)

Такой вариант будет ловить любые строки

pyuser

Ноя. 2, 2011 01:15:44

jcrow
Кроме нормальных случаев, он может быть None, или str c текстом ‘\n’.

попробуйте строку

import lxml.html

заменить на

from lxml import etree

тогда Ваша строка

self.html = lxml.html.document_fromstring(html)

преобразуется в две строки

parser = etree.HTMLParser(remove_blank_text=True, strip_cdata=False)
self.html = etree.fromstring(html, parser)

строка обхода, в этом случае, изменится на

for item in self.html.iterfind(".//a"):