Форум сайта python.su
Бодрого времени всем! Появилось в наличии страшное желание освоить Python. Пара дней работы (знакомства) и появилось немного кода. Хотелось бы услышать мнение более маститых товарищей. Код в студию:
from optparse import OptionParser
from re import search
from urllib import urlopen
from urlparse import urlparse
from subprocess import call
from os import devnull
def get_options():
"""Get command-line options"""
parser = OptionParser()
parser.add_option('-l', '--links')
parser.add_option('-d', '--dir', default='/media/sda5/tmp/images')
(options, args) = parser.parse_args()
return options
def main():
"""The main function
This function uses a string of HTML tags (<a>text</a> <a><img /></a>)
as --links option. aria2 is a default downloader. And don't forget to
change the download directory (--dir).
"""
options = get_options()
if options.links:
links = options.links.split('</a> <a')
total = len(links)
dnull = open(devnull, 'w')
strings = [
'\n{0} from {1} ({2}) \nGetting the link:',
'OK ({0})\nDownloading the image:',
'-d {0}',
'OK\n',
'ERROR\n',
'http://{0}/{1}',
'Error opening the web page, try again? (y or n)',
]
if total == 0:
print 'total == 0'
return 0
for (key, link) in enumerate(links):
matches = search('href="(.+?)"', link)
uri = matches.group(1)
print strings[0].format(key + 1, total, uri),
uopen = urlopen(uri)
content = ''
uri_parsed = urlparse(uri)
try_again = True
while content == '' and try_again:
content = uopen.read()
if content == '':
key = raw_input(strings[6])
if key == 'y':
try_again = True
else:
try_again = False
if 'imageshack' in uri_parsed.netloc:
matches = search('<link rel="image_src" href="(.+?)"', content)
uri = matches.group(1)
elif 'imagevenue' in uri_parsed.netloc:
matches = search('SRC="(.+?)"', content)
uri = strings[5].format(uri_parsed.netloc, matches.group(1))
print strings[1].format(uri),
return_code = call(['aria2c', uri, strings[2].format(options.dir)],
stdout=dnull)
if return_code == 0:
print strings[3],
else:
print strings[4],
return 0
if __name__ == '__main__':
main()
Отредактировано CHAOS (Сен. 8, 2012 14:07:34)
Офлайн
А чего оно делает?
Офлайн
В общем я усилием мозга таки осилил назначение этого.
Начнемс:
1. Уберите strings. Из-за наличия этого списка код нечитаем.
2. Комментарий к main тоже непонятен. Напишите что оно делает.
3. Вместо try_again и key используйте одну переменную try_again с начальным значением ‘y’. Цикл будет выглядеть как while content == ‘' and try_again == ’y', а выкрутасов с проверкой ввода не будет.
4. if return_code == 0 не pythonic. Используйте просто if return_code.
5. Для импортируемых сущностей, которые используются в коде один раз я бы в начале импортировал только модуль, а в коде указывал модуль.сущность. По-моему так читается проще.
6. Если aria2c просто даунлоадит, то зачем он вам нужен? У вас же есть urllib.
Отредактировано (Ноя. 19, 2011 21:25:50)
Офлайн
Спасибо, Ed, за комментарии, за наводки. Сейчас пойду чинить, но сперва хотел спросить. Пункт 5. Я импортировал сущности, а не модули только для экономии ресурсов, хотя читабельность от этого упала, это верно. Вопрос лишь в том, есть ли в такой экономии вообще смысл?
Офлайн
Для этого скрипта на мой взгляд не имеет. Если хотите улучшить производительность, то я бы смотрел в сторону параллелизации даунлоадов. Но для начала нужно разобраться что делает ваш скрипт и улучшить то, что есть. Так что не стесняйтесь, показывайте следующий вариант здесь - улучшим.
С точки зрения читабельности я бы предпочел re.search просто search-у, поскольку оно места больше не занимает, но гораздо понятнее читается. subprocess.call тоже не завредит. os.devnull из той же оперы. А вот OptionParser, urlopen и urlparse я бы оставил как есть, поскольку они менее generic, а urlparse.urlparse выглядит длиннее, но читабельности не прибавляет, даже скорее раздражает, смотрится как масло масляное.
Если хотите разобраться как работают импорты в Python, то могу порекомендовать статьи Андрея Светлова на эту тему: http://asvetlov.blogspot.com/2010/05/blog-post.html
Офлайн
Воспользовался Вашими советами, Ed.
strings переименовал в i18n, чтобы было понятнее, и убрал из основной функции. Лучше всего, я думаю, строки убрать в отдельный файл, но в рамках данного скрипта это бессмысленно.
Добавил комментарии, куда только смог, хотя мой английский - это хромая лошадь.
Убрал aria2c. Действительно, зачем она там была нужна?
Спасибо еще раз, Ed, за конструктивную критику. Благодаря доработкам код стал проще для понимания, меньше, уже, избавился от лишнего веса:) Обновленный код:
import re
import os
from urllib import urlopen, urlretrieve
from optparse import OptionParser
from urlparse import urlparse
i18n = [
'\n{0} from {1} ({2})\nGetting the page...',
'Downloading an image...',
'OK',
'ERROR. Try again? (y or n)',
'http://{0}/{1}',
'URI is oblivious',
'ERROR',
]
def get_options():
"""Get command-line options."""
parser = OptionParser()
parser.add_option('-l', '--links')
parser.add_option('-d', '--dir', default='/media/sda5/tmp/images')
(options, args) = parser.parse_args()
return options
def main():
"""This function gets URIs from href attributes of the --links option,
which must be a basic HTML code. Loading pages by URIs.
Finds links to images and downloading them.
"""
options = get_options()
if options.links is None:
return 1
matches = re.findall('href="(.+?)"', options.links)
total = len(matches)
if total == 0:
return 1
if options.dir[-1] != '/':
options.dir = ''.join((options.dir, '/'))
for (key, uri) in enumerate(matches):
print i18n[0].format(key + 1, total, uri)
uri_parsed = urlparse(uri)
try_again = 'y'
try:
uopen = urlopen(uri)
except IOError:
print i18n[5]
continue
while try_again == 'y':
uopen = urlopen(uri)
content = uopen.read()
uopen.close()
if content == '':
try_again = raw_input(i18n[3])
else:
break
if 'imageshack' in uri_parsed.netloc:
matches = re.search('<link rel="image_src" href="(.+?)"',
content)
uri = matches.group(1)
elif 'imagevenue' in uri_parsed.netloc:
matches = re.search('SRC="(.+?)"', content)
uri = i18n[4].format(uri_parsed.netloc, matches.group(1))
print i18n[1].format(uri)
uri_parsed = urlparse(uri)
file_name = uri_parsed.path.split('/')
(down_name, headers) = urlretrieve(uri,
''.join((options.dir, file_name[-1])))
if os.path.isfile(down_name):
print i18n[2]
else:
print i18n[6]
return 0
if __name__ == '__main__':
main()
Отредактировано CHAOS (Янв. 6, 2013 19:00:05)
Офлайн
Продолжим.
1. Я имел в виду не переименование strings, а убирание этого списка, как класса.
Как по-вашему что лучше читается:
print i18n.format(key + 1, total, uri) или print “\n{0} from {1} ({2})\nGetting the page…”.format(key + 1, total, uri) ?
Просто используйте строки прямо в коде, никуда их не нужно выносить. Они добавляют читабельности.
2. Если args вам не нужен, то можно делать так: (options, _args) = parser.parse_args()
Тогда pylint не будет ругаться на неиспользованную переменную.
3. default для -d какой-то странный. Я бы сделал просто текущий каталог. Это обычный ожидаемый эффект для скриптов такого типа. Для того же, чтобы найти куда ваш скрипт положил результат нужно будет смотреть в сорцы, что не гуд.
4. Вместо -l я бы попользовался args. Ваша строка - это не опция, а аргумент.
5. Зачем передавать строку в виде <a href=“bla-bla”>, а потом регекспами выдирать это bla-bla? проще просто передать туда “bla-bla”, тогда и пользоваться будет проще и регекспы не нужны.
6.
total = len(matches)
if total == 0:
return 1
if not matches:
return 1
if content == '':
try_again = raw_input(i18n[3])
else:
break
if not content:
try_again = raw_input("ERROR. Try again? (y or n)")
Офлайн
Доброго времени, Ed!
По поводу первого пункта. Строки вынесены в массив для интернационализации. Если же их вставить непосредственно в код, то как тогда быть с переводом?
И восьмой пункт. Если мы уберем else, то получим бесконечный цикл. Правильно ли я все понял?
Офлайн
Здравствуйте, все!
С интернационализацией все на самом деле просто - надо использовать gettext. По поводу пятого пункта. Эта программа должна работать в паре с Linkshop - онлайновым старшим братом, который на выходе дает HTML-строку. Остальные претензии вроде тоже учтены. Код:
# -*- coding: utf-8 -*-
"""Не секрет, что сервис imagevenue.com меняет адрес картинки каждый раз
при обновления страницы. Не так-то просто загрузить несколько картинок
сразу в данной ситуации. Эта программа призвана упростить данный процесс.
Также с ее помощью можно загружать материалы с imageshack.us и по прямым
ссылкам. Пример:
linkshop.py -l '<a href="imagevenue.com"></a><a href...' -d /ваш-каталог
"""
import re
import os
from urllib import urlopen, urlretrieve
from optparse import OptionParser
from urlparse import urlparse
def get_options():
"""Получает опции и аргументы командной строки."""
parser = OptionParser()
parser.add_option('-d', '--dir', default='')
return parser.parse_args()
def main():
"""Функция получает адреса страниц из href-атрибутов первого аргумента,
который должен быть обычным HTML-кодом. Используя адреса,
загружает страницы, находит там картинки и их тоже загружает.
"""
(options, args) = get_options()
if not args:
return 1
matches = re.findall('href="(.+?)"', args[0])
total = len(matches)
if not matches:
return 1
if options.dir and options.dir[-1] != '/':
options.dir = os.path.join(options.dir, '/')
for (key, url) in enumerate(matches):
string = '\n{0} из {1} ({2})\nПолучение страницы...'
print string.format(key + 1, total, url)
url_parsed = urlparse(url)
try:
url_open = urlopen(url)
except IOError:
print 'Неправильная ссылка'
continue
try_again = 'y'
while try_again == 'y':
url_open = urlopen(url)
content = url_open.read()
url_open.close()
if not content:
try_again = raw_input('ERROR. Попробовать еще раз? (y or n)')
else:
break
if 'imageshack' in url_parsed.netloc:
matches = re.search('<link rel="image_src" href="(.+?)"',
content)
url = matches.group(1)
elif 'imagevenue' in url_parsed.netloc:
matches = re.search('SRC="(.+?)"', content)
url = 'http://{0}/{1}'.format(url_parsed.netloc, matches.group(1))
print 'Загрузка картинки...'
url_parsed = urlparse(url)
file_name = url_parsed.path.split('/')
(down_name, _headers) = urlretrieve(url,
os.path.join(options.dir, file_name[-1]))
if os.path.isfile(down_name):
print 'OK'
else:
print 'ERROR'
return 0
if __name__ == '__main__':
main()
Отредактировано CHAOS (Янв. 6, 2013 19:01:28)
Офлайн
Вы как-то надолго исчезаете. Я думал уже все, кончилась ваша терпелка. Ан нет :)
1. Тихий выход в случае отсутствия аргументов или совпадений - это не очень юзер-френдли. У OptionParser-а есть вроде usage, error, help и т.д. Вполне уместно будет их попользовать.
2. total = len(matches) не нужен. достаточно будет print string.format(key + 1, len(matches), url)
3. Непонятно назначение этого:
if options.dir and options.dir[-1] != '/':
options.dir = os.path.join(options.dir, '/')
Офлайн