Suggested:
#!/usr/bin/python
import os
import requests
from bs4 import BeautifulSoup as bs
from pathlib import Path
from shutil import copyfileobj
MAX_PAGES = 1 # Max. number of pages is 41
SAVE_DIRECTORY = Path('fox_backgrounds')
BASE_URL = 'http://www.thefoxisblack.com/category/the-desktop-wallpaper-project/page'
RESOLUTIONS = {
'1280x800', '1440x900', '1680x1050', '1920x1200', '2560x1440',
'iphone', 'iphone-5', 'iphone6', 'iphone-6-plus', 'iphone6plus',
'ipad'
}
def fetch_url(url):
return requests.get(url).text
def clip_part(href):
return href.rpartition('/')[-1]
def save_image(href):
part = clip_part(href)
print(f' Downloading: {part}')
fn = SAVE_DIRECTORY / part
with requests.get(href, stream=True) as response, \
open(fn, 'wb') as output:
copyfileobj(response.raw, output)
def get_images_from_page(url):
html = fetch_url(url)
soup = bs(html, 'html.parser')
for link in soup.find_all('a', class_='btn_download'):
href = link['href']
if any(href.endswith(f'-{res}.jpg') for res in RESOLUTIONS):
save_image(href)
else:
print(f'Unknown resolution {href}')
def make_dir():
os.makedirs(SAVE_DIRECTORY, exist_ok=True)
def get_backgrounds():
make_dir()
for page in range(1, MAX_PAGES+1):
print(f'Fetching page {page}...')
get_images_from_page(f'{BASE_URL}{page}')
def main():
get_backgrounds()
if __name__ == '__main__':
main()
Comments:
- You initialized
SAVE_DIRECTORY
but then use it for creation and not file write
RESOLUTIONS
should be a set, or maybe a tuple
clip_url
is slightly misleading; it returns a URL part and not a whole URL
- It's best if you have a prompt before downloading the index page; otherwise it hangs without the user knowing what's happening
- Stream your content so that you don't use up memory for big files
- Your
RESOLUTIONS
check is a little puzzling. Maybe it's a validation step? But if it's a validation step, you silently fail instead of printing a warning. Also, you keep iterating even after you've found the correct resolution. I rewrote this to just check the current resolution, and also be a little bit more careful about where it's seen in the filename.
range(0, ...)
is redundant, but for your use case you're better off with range(1
anyway.
rpartition
does basically the same thing as what you wrote, but doesn't require any fancy array slicing
- Don't call
clip_path
twice
Edit
The following version makes sane use of generators so that the iteration functions only need to know about their iteration, and not the inner business logic.
Also, your resolution check needs to be case-insensitive for many of the files on the site; and the site has gif and png images as well as jpg. You were missing some resolutions and some alternate iPhone spellings. I don't think that it's worth doing a resolution check at all, especially given these edge cases, but I left it in.
#!/usr/bin/python
import os
import requests
from bs4 import BeautifulSoup as bs
from pathlib import Path
from shutil import copyfileobj
MAX_PAGES = 1 # Max. number of pages is 41
SAVE_DIRECTORY = Path('fox_backgrounds')
BASE_URL = 'http://www.thefoxisblack.com/category/the-desktop-wallpaper-project/page'
RESOLUTIONS = {
'1280x800', '1440x900', '1680x1050', '1920x1200', '2560x1440', '3840x2400',
'iphone', 'iphone5', 'iphone-5', 'iphone6', 'iphone-6-plus', 'iphone6plus', 'iphone6-plus',
'ipad'
}
def clip_part(href):
return href.rpartition('/')[-1]
def save_image(href):
part = clip_part(href)
print(f' Downloading: {part}')
fn = SAVE_DIRECTORY / part
with requests.get(href, stream=True) as response, \
open(fn, 'wb') as output:
copyfileobj(response.raw, output)
def urls_from_page(url):
soup = bs(requests.get(url).text, 'html.parser')
for link in soup.find_all('a', class_='btn_download'):
href = link['href']
if any(href.lower().contains(f'-{res}.') for res in RESOLUTIONS):
yield href
else:
print(f'Unknown resolution {href}')
def make_dir():
os.makedirs(SAVE_DIRECTORY, exist_ok=True)
def all_urls():
for page in range(1, MAX_PAGES+1):
print(f'Fetching page {page}...')
yield from urls_from_page(f'{BASE_URL}{page}')
def main():
make_dir()
for url in all_urls():
save_image(url)
if __name__ == '__main__':
main()