Fix issue 68 #69
Fix issue 68 #69
Conversation
| if re.match(r'^tagmanifest-.+\.txt$', filename): | ||
| continue | ||
| #remove everything up to the bag_dir directory | ||
| p=join(dirName, filename) |
There was a problem hiding this comment.
This should be p = following PEP-8.
| @@ -784,6 +784,15 @@ def _make_tagmanifest_file(alg, bag_dir): | |||
| for digest, filename in checksums: | |||
| tagmanifest.write('%s %s\n' % (digest, filename)) | |||
|
|
|||
| def _find_tag_files(bag_dir): | |||
There was a problem hiding this comment.
I think the Python community has increasingly discouraged the convention of using _ prefixes like this, but there's precedent in the other functions in this file. I'd be inclined to remove it.
| @@ -784,6 +784,15 @@ def _make_tagmanifest_file(alg, bag_dir): | |||
| for digest, filename in checksums: | |||
| tagmanifest.write('%s %s\n' % (digest, filename)) | |||
|
|
|||
| def _find_tag_files(bag_dir): | |||
| for dirName, subdirList, fileList in os.walk(bag_dir): | |||
There was a problem hiding this comment.
PEP-8 encourages dir_name, etc. I generally prefer to use plural names like subdirectories, filenames, rather than a list suffix but am ±0 on that.
| for dirName, subdirList, fileList in os.walk(bag_dir): | ||
| if not re.match(r'.*data$', dirName): | ||
| for filename in fileList: | ||
| if re.match(r'^tagmanifest-.+\.txt$', filename): |
There was a problem hiding this comment.
Rather than using a file regex, this could be faster and potentially easier to read as if filename.startswith('tagmanifest-')
| continue | ||
| #remove everything up to the bag_dir directory | ||
| p=join(dirName, filename) | ||
| yield p[len(bag_dir)+1:] |
There was a problem hiding this comment.
Stylistically, this should have spaces around operators (len(bag_dir) + 1) following PEP-8.
Logically, if I'm understanding it correctly this could also be written as yield os.path.relpath(p, start=bag_dir)
There was a problem hiding this comment.
cool, didn't know that this existed. Done
| @@ -267,6 +267,37 @@ def test_validate_optional_tagfile(self): | |||
| os.remove(j(tagdir, "tagfile")) | |||
| bag = bagit.Bag(self.tmpdir) | |||
| self.assertRaises(bagit.BagValidationError, self.validate, bag) | |||
|
|
|||
There was a problem hiding this comment.
This adds trailing whitespace, which makes diffs noisier
Shoudl fix issue #68