LibraryOfCongress / bagit-python

Work with BagIt packages from Python.
http://libraryofcongress.github.io/bagit-python
218 stars 85 forks source link

Fix for self.algorithms duplication issue in _load_manifests. #103

Closed mikedarcy closed 6 years ago

mikedarcy commented 6 years ago

When the bag._load_manifests function loops looking for tagmanifest- and manifest- files to determine the checksum algorithms currently in use, it calls self.algorithms.append(alg), which generates duplicate algorithm identifier list items in self.algorithms; once for the tagmanifest- files and then again for the manifest- files.

Additionally, the more calls to _load_manifests that are made on a given bag instance, the more the list grows. Calling bag.save causes this list to grow for every invocation, since at the end of the function self._load_manifests is called in order to reload the modified manifests. This also causes the bag.save function to invoke _make_tagmanifest_file unnecessarily for every duplicate list entry. You can see this behavior in action during the unit test function test_save_manifests.

The one-liner fix in this PR just checks for the existence of alg in self.algorithms before performing the append. Alternatively, I suppose self.algorithms could just be converted to (or wrapped with) a set, which is probably more pythonic.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.05%) to 83.211% when pulling deea5960ef4b7b8bfbea6e8858b1d5316bb4569b on mikedarcy:master into 1aa5090fa6124a1cd0c675fe670eb0ee38fb151b on LibraryOfCongress:master.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.05%) to 83.211% when pulling deea5960ef4b7b8bfbea6e8858b1d5316bb4569b on mikedarcy:master into 1aa5090fa6124a1cd0c675fe670eb0ee38fb151b on LibraryOfCongress:master.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.05%) to 83.211% when pulling deea5960ef4b7b8bfbea6e8858b1d5316bb4569b on mikedarcy:master into 1aa5090fa6124a1cd0c675fe670eb0ee38fb151b on LibraryOfCongress:master.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.05%) to 83.211% when pulling deea5960ef4b7b8bfbea6e8858b1d5316bb4569b on mikedarcy:master into 1aa5090fa6124a1cd0c675fe670eb0ee38fb151b on LibraryOfCongress:master.