bagit-python icon indicating copy to clipboard operation
bagit-python copied to clipboard

self.path shouldn't be None?

Open fkloft opened this issue 2 years ago • 1 comments

From bagit.py:

class Bag(object):
    # [...]
    def __init__(self, path=None):
        # [...]
        self.path = abspath(path)
        if path:
            # if path ends in a path separator, strip it off
            if path[-1] == os.sep:
                self.path = path[:-1]
            self._open()

The path argument defaults to None, but either way, the __init__ method probably doesn't do what's intended.

If path is None, the call to abspath (from os.path) fails with a TypeError. If path is not None, self.path is possibly overridden by self.path = path[:-1]. It should be noted that os.path.abspath already removes the trailing path separator, so the check shouldn't be necessary. If there was a trailing path separator, self.path suddenly is not an absolute path any more.

os.path.abspath only leaves the trailing separator if path == "/". Although I guess there should be no use case where you would want to create a bag at the filesystem root directory.

I noticed this when I was passing a pathlib.Path object to Bag(). I assumed it would work correctly as os.path.abspath can handle PathLike objects, but the additional check for a trailing path separator broke because pathlib.Path objects are not subscriptable.

fkloft avatar Apr 13 '23 11:04 fkloft

Is there even a use case for self.path to be None?

I would guess no; to create new bags, there's make_bag().

fkloft avatar Apr 13 '23 12:04 fkloft