Merge pull request #64 from Hwesta/code-cleanup

Code cleanup

Author: johnscancella

bagit.py

@@ -31,11 +31,11 @@
     % bagit.py --help
 """
 
+import argparse
 import codecs
 import hashlib
 import logging
 import multiprocessing
-import optparse
 import os
 import re
 import signal
@@ -247,7 +247,7 @@ def compare_fetch_with_fs(self):
     def payload_files(self):
         payload_dir = os.path.join(self.path, "data")
 
-        for dirpath, dirnames, filenames in os.walk(payload_dir):
+        for dirpath, _, filenames in os.walk(payload_dir):
             for f in filenames:
                 # Jump through some hoops here to make the payload files come out
                 # looking like data/dir/file, rather than having the entire path.
@@ -344,7 +344,7 @@ def fetch_entries(self):
                     yield (parts[0], parts[1], parts[2])
 
     def files_to_be_fetched(self):
-        for f, size, path in self.fetch_entries():
+        for f, _, _ in self.fetch_entries():
             yield f
 
     def has_oxum(self):
@@ -521,7 +521,7 @@ def _init_worker():
                 finally:
                     try:
                         pool.terminate()
-                    except:
+                    except Exception:
                         # we really don't care about any exception in terminate()
                         pass
         # Any unhandled exceptions are probably fatal
@@ -877,42 +877,45 @@ def _decode_filename(s):
 
 # following code is used for command line program
 
-class BagOptionParser(optparse.OptionParser):
-    def __init__(self, *args, **opts):
+class BagArgumentParser(argparse.ArgumentParser):
+    def __init__(self, *args, **kwargs):
         self.bag_info = {}
-        optparse.OptionParser.__init__(self, *args, **opts)
+        argparse.ArgumentParser.__init__(self, *args, **kwargs)
 
 
-def _bag_info_store(option, opt, value, parser):
-    opt = opt.lstrip('--')
-    opt_caps = '-'.join([o.capitalize() for o in opt.split('-')])
-    parser.bag_info[opt_caps] = value
+class BagHeaderAction(argparse.Action):
+    def __call__(self, parser, _, values, option_string=None):
+        opt = option_string.lstrip('--')
+        opt_caps = '-'.join([o.capitalize() for o in opt.split('-')])
+        parser.bag_info[opt_caps] = values
 
 
-def _make_opt_parser():
-    parser = BagOptionParser(usage='usage: %prog [options] dir1 dir2 ...')
-    parser.add_option('--processes', action='store', type="int",
-                      dest='processes', default=1,
+def _make_parser():
+    parser = BagArgumentParser()
+    parser.add_argument('--processes', type=int, dest='processes', default=1,
                       help='parallelize checksums generation and verification')
-    parser.add_option('--log', action='store', dest='log', help='The name of the log file')
-    parser.add_option('--quiet', action='store_true', dest='quiet')
-    parser.add_option('--validate', action='store_true', dest='validate')
-    parser.add_option('--fast', action='store_true', dest='fast')
+    parser.add_argument('--log', help='The name of the log file')
+    parser.add_argument('--quiet', action='store_true')
+    parser.add_argument('--validate', action='store_true')
+    parser.add_argument('--fast', action='store_true')
 
     # optionally specify which checksum algorithm(s) to use when creating a bag
     # NOTE: could generate from checksum_algos ?
-    parser.add_option('--md5', action='append_const', dest='checksum', const='md5',
+    parser.add_argument('--md5', action='append_const', dest='checksum', const='md5',
                       help='Generate MD5 manifest when creating a bag (default)')
-    parser.add_option('--sha1', action='append_const', dest='checksum', const='sha1',
+    parser.add_argument('--sha1', action='append_const', dest='checksum', const='sha1',
                       help='Generate SHA1 manifest when creating a bag')
-    parser.add_option('--sha256', action='append_const', dest='checksum', const='sha256',
+    parser.add_argument('--sha256', action='append_const', dest='checksum', const='sha256',
                       help='Generate SHA-256 manifest when creating a bag')
-    parser.add_option('--sha512', action='append_const', dest='checksum', const='sha512',
+    parser.add_argument('--sha512', action='append_const', dest='checksum', const='sha512',
                       help='Generate SHA-512 manifest when creating a bag')
 
     for header in STANDARD_BAG_INFO_HEADERS:
-        parser.add_option('--%s' % header.lower(), type="string",
-                          action='callback', callback=_bag_info_store)
+        parser.add_argument('--%s' % header.lower(), type=str,
+                          action=BagHeaderAction)
+
+    parser.add_argument('directory', nargs='+', help='directory to make a bag from')
+
     return parser
 
 
@@ -929,24 +932,24 @@ def _configure_logging(opts):
 
 
 def main():
-    opt_parser = _make_opt_parser()
-    opts, args = opt_parser.parse_args()
+    parser = _make_parser()
+    args = parser.parse_args()
 
-    if opts.processes < 0:
-        opt_parser.error("number of processes needs to be 0 or more")
+    if args.processes < 0:
+        parser.error("number of processes needs to be 0 or more")
 
-    _configure_logging(opts)
+    _configure_logging(args)
 
     rc = 0
-    for bag_dir in args:
+    for bag_dir in args.directory:
 
         # validate the bag
-        if opts.validate:
+        if args.validate:
             try:
                 bag = Bag(bag_dir)
                 # validate throws a BagError or BagValidationError
-                bag.validate(processes=opts.processes, fast=opts.fast)
-                if opts.fast:
+                bag.validate(processes=args.processes, fast=args.fast)
+                if args.fast:
                     LOGGER.info("%s valid according to Payload-Oxum", bag_dir)
                 else:
                     LOGGER.info("%s is valid", bag_dir)
@@ -957,9 +960,9 @@ def main():
         # make the bag
         else:
             try:
-                make_bag(bag_dir, bag_info=opt_parser.bag_info,
-                         processes=opts.processes,
-                         checksum=opts.checksum)
+                make_bag(bag_dir, bag_info=parser.bag_info,
+                         processes=args.processes,
+                         checksum=args.checksum)
             except Exception as exc:
                 LOGGER.error("Failed to create bag in %s: %s", bag_dir, exc, exc_info=True)
                 rc = 1

test.py

@@ -181,7 +181,7 @@ def test_allow_extraneous_files_in_base(self):
         bag = bagit.make_bag(self.tmpdir)
         self.assertTrue(self.validate(bag))
         f = j(self.tmpdir, "IGNOREFILE")
-        with open(f, 'w') as whatever:
+        with open(f, 'w'):
             self.assertTrue(self.validate(bag))
 
     def test_allow_extraneous_dirs_in_base(self):
@@ -206,7 +206,7 @@ def test_missing_manifest_raises_error(self):
     def test_mixed_case_checksums(self):
         bag = bagit.make_bag(self.tmpdir)
         hashstr = {}
-        #Extract entries only for the payload and ignore
+        # Extract entries only for the payload and ignore
         # entries from the tagmanifest file
         for key in bag.entries.keys():
             if key.startswith('data' + os.sep):
@@ -218,7 +218,7 @@ def test_mixed_case_checksums(self):
         with open(j(self.tmpdir, "manifest-md5.txt"), "w") as m:
             m.write(manifest)
 
-        #Since manifest-md5.txt file is updated, re-calculate its
+        # Since manifest-md5.txt file is updated, re-calculate its
         # md5 checksum and update it in the tagmanifest-md5.txt file
         hasher = hashlib.new('md5')
         with open(j(self.tmpdir, "manifest-md5.txt"), "r") as manifest:
@@ -300,7 +300,7 @@ def tearDown(self):
 
     def test_make_bag(self):
         info = {'Bagging-Date': '1970-01-01', 'Contact-Email': '[email protected]'}
-        bag = bagit.make_bag(self.tmpdir, bag_info=info)
+        bagit.make_bag(self.tmpdir, bag_info=info)
 
         # data dir should've been created
         self.assertTrue(os.path.isdir(j(self.tmpdir, 'data')))
@@ -340,7 +340,7 @@ def test_make_bag(self):
         self.assertTrue('6a5090e27cb29d5dda8a0142fbbdf37e bag-info.txt' in tagmanifest_txt)
 
     def test_make_bag_sha1_manifest(self):
-        bag = bagit.make_bag(self.tmpdir, checksum=['sha1'])
+        bagit.make_bag(self.tmpdir, checksum=['sha1'])
         # check manifest
         self.assertTrue(os.path.isfile(j(self.tmpdir, 'manifest-sha1.txt')))
         with open(j(self.tmpdir, 'manifest-sha1.txt')) as m:
@@ -352,7 +352,7 @@ def test_make_bag_sha1_manifest(self):
         self.assertTrue('db49ef009f85a5d0701829f38d29f8cf9c5df2ea  data/si/4011399822_65987a4806_b_d.jpg' in manifest_txt)
 
     def test_make_bag_sha256_manifest(self):
-        bag = bagit.make_bag(self.tmpdir, checksum=['sha256'])
+        bagit.make_bag(self.tmpdir, checksum=['sha256'])
         # check manifest
         self.assertTrue(os.path.isfile(j(self.tmpdir, 'manifest-sha256.txt')))
         with open(j(self.tmpdir, 'manifest-sha256.txt')) as m:
@@ -363,7 +363,7 @@ def test_make_bag_sha256_manifest(self):
         self.assertTrue('45d257c93e59ec35187c6a34c8e62e72c3e9cfbb548984d6f6e8deb84bac41f4  data/si/4011399822_65987a4806_b_d.jpg' in manifest_txt)
 
     def test_make_bag_sha512_manifest(self):
-        bag = bagit.make_bag(self.tmpdir, checksum=['sha512'])
+        bagit.make_bag(self.tmpdir, checksum=['sha512'])
         # check manifest
         self.assertTrue(os.path.isfile(j(self.tmpdir, 'manifest-sha512.txt')))
         with open(j(self.tmpdir, 'manifest-sha512.txt')) as m:
@@ -378,7 +378,7 @@ def test_make_bag_unknown_algorithm(self):
 
     def test_make_bag_with_data_dir_present(self):
         os.mkdir(j(self.tmpdir, 'data'))
-        bag = bagit.make_bag(self.tmpdir)
+        bagit.make_bag(self.tmpdir)
 
         # data dir should now contain another data dir
         self.assertTrue(os.path.isdir(j(self.tmpdir, 'data', 'data')))
@@ -425,7 +425,7 @@ def test_garbage_in_bagit_txt(self):
         self.assertRaises(bagit.BagValidationError, bagit.Bag, self.tmpdir)
 
     def test_make_bag_multiprocessing(self):
-        bag = bagit.make_bag(self.tmpdir, processes=2)
+        bagit.make_bag(self.tmpdir, processes=2)
         self.assertTrue(os.path.isdir(j(self.tmpdir, 'data')))
 
     def test_multiple_meta_values(self):
@@ -437,7 +437,7 @@ def test_multiple_meta_values(self):
 
     def test_default_bagging_date(self):
         info = {'Contact-Email': '[email protected]'}
-        bag = bagit.make_bag(self.tmpdir, bag_info=info)
+        bagit.make_bag(self.tmpdir, bag_info=info)
         with open(j(self.tmpdir, 'bag-info.txt')) as bi:
             bag_info_txt = bi.read()
         self.assertTrue('Contact-Email: [email protected]' in bag_info_txt)
@@ -468,7 +468,7 @@ def test_payload_permissions(self):
         new_perms = perms | stat.S_IWOTH
         self.assertTrue(perms != new_perms)
         os.chmod(self.tmpdir, new_perms)
-        bag = bagit.make_bag(self.tmpdir)
+        bagit.make_bag(self.tmpdir)
         payload_dir = j(self.tmpdir, 'data')
         self.assertEqual(os.stat(payload_dir).st_mode, new_perms)
 
@@ -525,7 +525,7 @@ def test_save_only_baginfo(self):
             nf.write('newfile')
         bag.info["foo"] = "bar"
         bag.save()
-        
+
         bag = bagit.Bag(self.tmpdir)
         self.assertEqual(bag.info["foo"], "bar")
         self.assertFalse(bag.is_valid())