When you encounter an ImportError it does not really help you debug the problem if you ignore it, just print the string "Import Error" and then fail at some random later point where that module is actually needed. This will generate a lot more error messages which are less clear than the normal ImportError!
Think about using argparse for your command line arguments. This will not allow you the shenanigans of asking three times for an existing directory name before offering to create it, but you can just add a flag for this:
import argparse
parser = argparse.ArgumentParser()
parser.add_argument("url", help="The base page where to search for PDF files.")
parser.add_argument("path", help="Where to save the files")
parser.add_argument("--create-directory", "-p", action="store_true", help="Create the directory if it does not exist")
locals().update(vars(parser.parse_args()))
if not os.path.isdir(path):
if create_directory:
os.makedirs(path)
else:
raise OSError("Directory {} does not exist. Use the '-p' flag to create it".format(path))
I used a trick here to update the locals dictionary, which is just the variables in the local scope. This way the variable url and path exist and have the right value. This magic works only outside of a function, inside you can use:
import argparse
parser = argparse.ArgumentParser()
parser.add_argument("url", help="The base page where to search for PDF files.")
parser.add_argument("path", help="Where to save the files")
parser.add_argument("--create-directory", "-p", action="store_true", help="Create the directory if it does not exist")
args = parser.parse_args()
if not os.path.isdir(args.path):
if create_directory:
os.makedirs(args.path)
else:
raise OSError("Directory {} does not exist. Use the '-p' flag to create it".format(args.path))
Use this argument parser by calling your script like:
$ python pdf_downloader.py http://url.to/pdfs.html path/to/save/files/to/
If you don't enter the two required arguments, an automatically generated usage message will be displayed:
$ python pdf_download.py
usage: pdf_download.py [-h] [-p] url path
pdf_download.py: error: too few arguments
$ python pdf_download.py --help
usage: pdf_download.py [-h] [-p] url path
positional arguments:
url The base page where to search for PDF files.
path Where to save the files
optional arguments:
-h, --help show this help message and exit
-p, --create-directory Create the directory if it does not exist
Interlude: Python has an official style guide, PEP8. One of the things it recommends is using a blank after a comma in a(n) (argument-)list, like (1, 2, 3). Another thing is to use lower_case for variable and function names. In all of the below code I fixed this.
Your url manipulation is probably better written as
base_url = '/'.join(url.split('/')[:-1])
And the separating trailing '/' can be added when actually building the full url. I would also put it into a new variable, just to make it clearer what this is.
Even better would be to just use the built-in os.path.dirname, since you already imported os:
base_url = os.path.dirname(url)
For the full url I would use str.format, which makes it a bit easier to read and which is also faster than multiple string additions (even though for one addition it does not matter).
full_url = "{}/{}".format(base_url, link.get('href'))
Instead of separately saving the urls to download and their names, only to zip it first and then iterate over the zipped container, just save the two together right away:
if full_url.endswith('.pdf'):
# print fullurl
name = soup1.select('a')[i].attrs['href']
urls.append((name, full_url))
pass is not what you think it is. It is a no-op that does nothing (you can use it as placeholder when you need some code block there because of indentation but don't have the code yet). What you mean is continue which directly goes to the next iteration of the loop.
You should never have a bare except clause. This will also catch e.g. CTRL-C if you run into an infinite loop and want to abort it! Use at least except Exception or better, figure out the actual exceptions you code would raise.
Also try to limit the scope of the except to the one call that might fail, if possible. At leas use except Exception as e and print the exception afterwards. Otherwise this information is lost.
Figuring out which urls to download and actually downloading them are two distinct tasks, which are ideal for functions. In addition I would add a main function for the rest and guard it with a if __name__ == "__main__": clause. The latter allows you to do in another script to just do from pdf_download import get_urls and reuse that function.
This results in the final code:
import urllib2
from bs4 import BeautifulSoup
from time import sleep
import os
import sys
import argparse
HEADERS = {'User-Agent': 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.106 Safari/537.36'}
def get_urls(url):
request = urllib2.Request(url, None, HEADERS)
soup = BeautifulSoup(urllib2.urlopen(request).read(), "html.parser")
soup.prettify()
urls = []
base_url = os.path.dirname(url)
for anchor in soup.findAll('a', href=True): #Going inside links
req = urllib2.Request(anchor['href'], None, HEADERS)
soup1 = BeautifulSoup(urllib2.urlopen(req).read(), "html.parser")
for i, link in enumerate(soup1.findAll('a')): # Download all pdf inside each link
full_url = "{}/{}".format(base_url, link.get('href'))
if full_url.endswith('.pdf'):
name = soup1.select('a')[i].attrs['href']
urls.append((name, full_url))
return urls
def download(urls, path):
old_dir = os.getcwd()
os.chdir(path)
for name, url in urls:
if os.path.isfile(name):
print name, "already exists, skipping..."
continue
try:
request = urllib2.Request(url)
res = urllib2.urlopen(request).read()
with open(name, 'wb') as pdf:
pdf.write(res)
print "Downloaded", name
except Exception as e:
print "Failed to download", name, ", because of", e
os.chdir(old_dir)
def main():
parser = argparse.ArgumentParser()
parser.add_argument("url", help="The base page where to search for PDF files.")
parser.add_argument("path", help="Where to save the files")
parser.add_argument("--create-directory", "-p", action="store_true", help="Create the directory if it does not exist")
args = parser.parse_args()
if not os.path.isdir(args.path):
if args.create_directory:
os.makedirs(args.path)
else:
raise OSError("Directory {} does not exist. Use the '-p' flag to create it".format(args.path))
download(get_urls(args.url), args.path)
if __name__ == "__main__":
main()
Here I made some more cosmetic changes, like using proper grammar in the notifications ("Downloaded file_name" instead of "file_name Downloaded"), reverting back to the original working directory after downloading all files, removing unnecessary comments and made headers a constant, because it is used in multiple functions now. I also deleted the sleep because it is usually not necessary (unless there is some flood protection on the website, maybe).
