Webscrape Podcast site for mp3s to (legally) add to collection

Question

History

I am new to Python and this is my first attempt. Utilizing a legal login (omitted from the code below) to the Bob and Tom VIP website, I have created this script to access their Full Show mp3. Using the couple of Python books I have and Googling StackExchange, I have been able to get this script to the rudimentary state, albeit working, it is in.

This is initially written to run in IDLE while testing and then to have the input portion removed to work via cron on my fileserver.

Performance

While walking through each individual show page and downloading the mp3, the script does not take long; I am using a single Session(). I would image extracting all the notes and links for each instance into a dict would increase the process, but it is still limited to the speed of the connection. I'm not sure if the iGate variable for the POST will allow multiple instances.

Note: username, password, and (possibly) identifying iGate value from myurl has been removed.

Code

"""
Webscraping Bob and Tom for Full Show and mp3.
"""
import requests
from bs4 import BeautifulSoup
from datetime import datetime, timedelta, date
from dateutil.rrule import DAILY, rrule, MO, TU, WE, TH, FR
import os
#import eyed3 #failed to install with Python devel ver

# set Bob and Tom login and basic information
usr = 'real usr removed'
pwd = 'real pwd removed'
myurl = 'https://members.bobandtom.com/pcd/egate?iGATE=real iGate removed'
login_url = 'https://members.bobandtom.com/pcd/eGateChk'
vip_url = 'http://vip.bobandtom.com/'
usr_agent = 'Mozilla/5.0 (Windows NT 6.1; Win64; x64) Chrome/62.0.3202.94'

# check which Operating System we are running on to set valid directory
if (os.name == 'nt'):
    mp3dir = 'z:\\Music\\0 BobandTom'
else:
    mp3dir = '/media/Music/0 BobandTom/'

# establish login session with VIP website
s = requests.Session()
hdrs = { 'User-Agent' : usr_agent }
s.headers.update(hdrs)
r = s.get(myurl)
soup = BeautifulSoup(r.content, 'html.parser')

# get iGate string
gate_val = soup.find('input', {'name' : 'iGate'}).get('value')
# establish payload for login
login_dict = { 'iGate' : gate_val,
               'iType' : 'MAIN',
               'iUserName' : usr,
               'iPassword' : pwd }
# establish headers for login
hdrs = { 'Host' : 'members.bobandtom.com',
         'Referrer' : myurl,
         'Content-type' : 'application/x-www-form-urlencoded' }
# post login information
r = s.post(login_url, headers = hdrs, data = login_dict)

# START the local determination
# puts files into sorted list from directory if they exist
mp3list = sorted([f for f in os.listdir(mp3dir)
                  if os.path.splitext(f)[1] == '.mp3'])

# set the next mp3 we want to dowload
if not mp3list:
    print("There are no MP3s in your directory.")
    mp3want = str(input("What MP3 do you wish to start with [format YYYYMMDD]? "))
    print("Attempting to download " + mp3want + ".mp3")
    start_date = date(int(mp3want[:4]), 
                  int(mp3want[4:6]), 
                  int(mp3want[-2:]))
else:
    mp3last = mp3list[-1] # last mp3 in our directory
    print("Attempting to dowload all MP3s after " + (mp3last) + ".")
    mp3want = mp3last.split(sep='.mp3', maxsplit=1)[0] # is str
    start_date = date(int(mp3want[:4]), 
                  int(mp3want[4:6]), 
                  int(mp3want[-2:])) + timedelta(days=1)
# set last mp3 to download (the day before today)
end_date = date.today() - timedelta(days=1)

# generate the dates from start to end (weekdays only)
def daterange(start_date, end_date):
    return rrule(DAILY, dtstart=start_date,
                 until=end_date, byweekday=(MO,TU,WE,TH,FR))

# create dates and put them in list (format: 2017/09/01/september-1-2017/)
date_links = []
for d_string in daterange(start_date, end_date):
    if (os.name == 'nt'):
        date_links.append(d_string.strftime("%Y/%m/%d/%B-%#d-%Y/").lower())
    else:
        date_links.append(d_string.strftime("%Y/%m/%d/%B-%-d-%Y/").lower())

# create list with full urls
showpage_htmls = [vip_url + date_link for date_link in date_links]

# START the d/l process
# within session, process through direct to show pages
for show_html in showpage_htmls:
    try:
        p_html = s.get(show_html)
        p_soup = BeautifulSoup(p_html.content, 'html.parser')

        # set title for SHOW NOTES
        title = ''
        title = p_soup.title.contents[0]

        # get SHOW NOTES chunk and remove unwanted characters (original mp3notes not changed)
        mp3notes = ''
        mp3notes = p_soup.find('div', {'class': 'module-text'}).find('div')
        mp3notes = str(title) + str('\n') + str(mp3notes).replace('<div>','').replace('<h2>','').replace('</h2>','\n').replace('<p>','').replace('<br/>\n','\n').replace('<br/>','\n').replace('</p>','').replace('</div>','').replace('\u2032','')

        # set basename, mp3named and mp3showtxt
        mp3basename = '{0}{1}{2}'.format(show_html.split('/')[3],show_html.split('/')[4],show_html.split('/')[5])
        if (os.name == 'nt'):
            mp3showtxt = mp3dir + '\\' + mp3basename + '.txt'
            mp3named = mp3dir + '\\' + mp3basename + '.mp3'
        else:
            mp3showtxt = mp3dir + '/' + mp3basename + '.txt'
            mp3named = mp3dir + '/' + mp3basename + '.mp3'

        # save show notes to local
        with open(mp3showtxt, 'w') as f:
            try:
                f.write(mp3notes)
                print("Show notes " + mp3basename + " saved.")
            except UnicodeEncodeError:
                print("A charmap encoding ERROR occurred.")
                print("Show notes for " + mp3basename + ".mp3 FAILED, but continuing")
            finally:
                f.close()
        # FIXME need eyed3 to set mp3 tags since B&T are lazy
        # get Full Show mp3 link
        mp3url = p_soup.find('a', href = True, string = 'Full Show').get('href')

        # get and save mp3
        r = requests.get(mp3url)
        with open(mp3named, 'wb') as f:
            f.write(r.content)
        print("Downloaded " + mp3basename + ".mp3.")
    except AttributeError:
        print(show_html + " did not exist as named.")

Expectations of review

• Suggestions for more efficient code, i.e. class or def usage(s).
• Suggestions for trapping no show notes (mp3showtxt) found and skipping that d/l.
• And of course any suggestions for improvement.

Thanks in advance for any and all input; it will help me continue learning.

Answer 1

There is a space for multiple improvements, here are some of the high-level ideas:

• the code itself is long and "complex" and should definitely be "cut" into pieces logically - apply "Extract Method" refactoring method

• "Extract Variable" might also be beneficial, e.g.:

   is_windows = os.name == 'nt'
   mp3dir = 'z:\\Music\\0 BobandTom' if is_windows else '/media/Music/0 BobandTom/'
  

• use a linting tool like flake8 or pylint to analyze your code statically and catch and address the code-style violations

Performance

• switching from html.parser to lxml would provide you an HTML parsing performance boost "for free":

  BeautifulSoup(p_html.content, 'lxml')
  

  lxml, of course, needs to be installed

• SoupStrainer might help to further improve the HTML parsing speed. Parse only the relevant part of the document
• you don't use a "session" here: r = requests.get(mp3url)

Nitpicks & Style Guide violations & Pythonic code

• from datetime import datetime import is unused, you can remove it
• remove extra parenthesis around the if conditions. For instance, os.name == 'nt' instead of (os.name == 'nt')
• I think you are overusing spaces in operators in expressions - (spaces before the } and after { are extra; or spaces around the = in keyword argument definitions are not needed as well). Find out more about proper use of spaces in the PEP8 style-guide

• you don't need to define and re-define title (same goes for mp3notes):

  title = ''
  title = p_soup.title.contents[0]
  

  just do:

  title = p_soup.title.contents[0]
  

• watch for the "continuation line under-indented for visual indent" PEP8 style-guide violation

• date_links might be defined in a list comprehension taking into account that you can and should define the datestring format before the loop ealier:

  DATETIME_FORMAT = "%Y/%m/%d/%B-%#d-%Y/" if is_windows else "%Y/%m/%d/%B-%-d-%Y/"
  date_links = [d_string.strftime(DATETIME_FORMAT).lower() 
                for d_string in daterange(start_date, end_date)]
  

Answer 2

Comments inline.

• usr = 'real usr removed'
  pwd = 'real pwd removed'
  

  Get these from a configuration file. Never store credentials in your code. Check out the configparser library.

• # check which Operating System we are running on to set valid directory
  if (os.name == 'nt'):
      mp3dir = 'z:\\Music\\0 BobandTom'
  else:
      mp3dir = '/media/Music/0 BobandTom/'
  

  You should look into using os.path.join() or pathlib.Path, and '0 BobandTom' should probably be its own variable so it's not duplicated. The if should just assign 'z:' or '/media' to a temporary variable or the first element of the list given to os.path.join() . Also look into raw strings -- r'embedded\backslashes\okay' to avoid leaning toothpick syndrome.

• mp3list = sorted([f for f in os.listdir(mp3dir)
      if os.path.splitext(f)[1] == '.mp3'])
  

  You never use most of mp3list. The only thing it's used for is to checkk whether there are any items, and then determine the largest item. Why not use max? Also, it's probably quicker to just check the last 4 characters of f using a string slice as in f[:-4] though that loses support for extensions of different length.

• # set the next mp3 we want to dowload
  

  Spell check

• mp3want = str(input("What MP3 do you wish to start with [format YYYYMMDD]? "))
  

  Given that input() already returns a str, what's the purpose of str() here? Also you should be planning ahead for how you are going to change this code to take from a batch file or whatever -- these settings should be determined by the main function (perhaps calling input()) and then passed in to this function as parameters.

• print("Attempting to download " + mp3want + ".mp3")
  print("Attempting to dowload all MP3s after " + (mp3last) + ".")
  

  You could use formatted strings here. print(f"Download {mp3want}.mp3") No need for the extra parens on mp3last. But these logging messages should really be communicated via the logging module as they are not part of the output of the program.

• mp3want = mp3last.split(sep='.mp3', maxsplit=1)[0] # is str
  

  The comment isn't helpful. Name the variable properly. Again it's probably easier to slice: mp3want = mp3last[:-4] or use os.path.splittext() as before.

• date_links = []
  for d_string in daterange(start_date, end_date):
      if (os.name == 'nt'):
          date_links.append(d_string.strftime("%Y/%m/%d/%B-%#d-%Y/").lower())
      else:
          date_links.append(d_string.strftime("%Y/%m/%d/%B-%-d-%Y/").lower())
  # create list with full urls
  showpage_htmls = [vip_url + date_link for date_link in date_links]
  

  • Duplicated code indicates there's a better way to do it.
  • Don't put if inside a loop if you can help it.
  • []/for/append is an anti-pattern. Just use a list comprehension.
  • Also d_string is not a string and should not be named such.
  • Isn't '/' an illegal character in NT filenames? And isn't it the directory separator on UNIX? Seems like a strange format string to use.

  • Suggestion:

    if os.name=='nt':
        formatstring='%Y/%m/%d/%B-%#d-%Y/'
    else:
        formatstring='%Y/%m/%d/%B-%-d-%Y/'
    showpage_htmls = [vip_url + d.strftime(formatstring) for d in daterange(start_date,end_date)]
    

• mp3notes = str(title) + str('\n') + str(mp3notes).replace('<div>','').replace('<h2>','').replace('</h2>','\n').replace('<p>','').replace('<br/>\n','\n').replace('<br/>','\n').replace('</p>','').replace('</div>','').replace('\u2032','')
  

  Line is too long, and too much repeated code. Put all these replacements in a data structure and loop over it to replace them. Also what's with the gratuitous use of str()?

• # set basename, mp3named and mp3showtxt
  

  The comment doesn't add any value. It just restates what the code says. What are these variables actually for?

• mp3basename = '{0}{1}{2}'.format(show_html.split('/')[3],show_html.split('/')[4],show_html.split('/')[5])
  

  Put show_html.split('/') in a temporary variable rather than recalculating it multiple times

• if (os.name == 'nt'):
      mp3showtxt = mp3dir + '\\' + mp3basename + '.txt'
      mp3named = mp3dir + '\\' + mp3basename + '.mp3'
  else:
      mp3showtxt = mp3dir + '/' + mp3basename + '.txt'
      mp3named = mp3dir + '/' + mp3basename + '.mp3'
  

  Again, os.path.join() or Path() is your friend.

• # save show notes to local
  with open(mp3showtxt, 'w') as f:
      try:
          f.write(mp3notes)
          print("Show notes " + mp3basename + " saved.")
      except UnicodeEncodeError:
          print("A charmap encoding ERROR occurred.")
          print("Show notes for " + mp3basename + ".mp3 FAILED, but continuing")
      finally:
          f.close()
  

  What about file/OS errors? Again these should be logging not print().