2
\$\begingroup\$

I am learning Python and I know that a lot of my future projects will be based on files and managing them. How does this look? Is anything in here redundant or overcomplicated?

import os
import zipfile
import shutil
from datetime import datetime as dt


class File:
    def __init__(self, path, datetime=True):
        self.path = path
        self.datetime = datetime

        self.abs_path = os.path.abspath(self.path)

        self._dt_manager = lambda prop: prop if not self.datetime else dt.fromtimestamp(prop)
        self._os_path = lambda func, path_=self.abs_path, *args, **kwargs: getattr(os.path, func)(path_, *args, **kwargs)

        self.readable = False
        self.data = None
        self.lines = None
        self.line_count = None

        try:
            with open(self.path, "r") as target:
                self.data = target.read()
                self.lines = self.data.splitlines()
                self.line_count = len(self.lines)
                self.readable = True

        except UnicodeDecodeError:
            pass

        self.size_in_bytes = self._os_path("getsize")
        self.size = self.format_size(self.size_in_bytes)

        self.created = self._dt_manager(self._os_path("getctime"))
        self.last_modified = self._dt_manager(self._os_path("getmtime"))
        self.last_accessed = self._dt_manager(self._os_path("getatime"))

        self.full_name = self._os_path("basename")
        self.name, self.extension = self._os_path("splitext", self.full_name)

        self.dir_path = self._os_path("dirname")
        self.dir_name = self._os_path("basename", self.dir_path)

    def __repr__(self):
        return "<File {}>".format(self.full_name)

    def __str__(self):
        return self.abs_path

    def __getitem__(self, index):
        return self.lines[index]

    def __iter__(self):
        return self.data

    def __eq__(self, other):
        return self.abs_path == (other.abs_path if isinstance(other, File) else other)

    def __len__(self):
        return self.line_count

    @staticmethod
    def format_size(size_in_bytes, suffix="B"):
        for unit in ["", "K", "M", "G", "T"]:
            if abs(size_in_bytes) < 1024.0:
                return str(size_in_bytes) + unit + suffix
            size_in_bytes /= 1024.0

        return str(size_in_bytes) + "Y" + suffix

    def unzip(self, location=None):
        if location is None:
            location = self.dir_path

        with zipfile.ZipFile(self.abs_path, "r") as target:
            return target.extractall(location)

    def rename(self, new_name):
        return os.rename(self.abs_path, new_name)

    def copy(self, location=None, new_name=None):
        if location is None:
            location = self.abs_path

        return shutil.copy(self.abs_path, location if not new_name else location + new_name)

    def duplicate(self, amount=1, splitter="-"):
        duplicate_name = lambda index: self.name + splitter + str(index) + self.extension

        if amount > 1:
            for i in range(0, amount):
                self.copy(duplicate_name(i))

            return None

        return self.copy(duplicate_name(0))

    def delete(self):
        return os.remove(self.abs_path)

    def find(self, query):
        return [line for line in self.lines if query in line]
\$\endgroup\$
1
\$\begingroup\$

Imports

  1. It's really uncommon to use the as part of the import. The only time I see it is in NumPy, which is a pretty un-Pythonic library.
  2. I recommend to only use import rather than from ... import statements. This is as it's harder to determine what has been imported from where.
  3. When importing datetime I find it best to only use import datetime. Given that one of the classes is called datetime it can lead to situations where it's unknown what datetime is without having to then look at the imports.

Class

I think you're on the right track, but you've added too much into your class. This is commonly referred to as the god-class anti-pattern.

I think it's a good idea, as pathlib implements a lot of the same stuff that you are doing. Whilst I can understand and would possibly encourage some extensions to the class, I don't think all your changes are great additions.

The following functions can all be replaced with pathlib in some form. Note that the entire of the bottom chunk can utilize Path.stat.

self.full_name = Path.name
self.name = Path.stem
self.extension = Path.suffix
self.dir_path = Path.parent
self.dir_name = Path.parent.name

Path.stat:
    datetime:
        self.created = os.path.getctime
        self.last_modified = os.path.getmtime
        self.last_accessed = os.path.getatime
    self.size_in_bytes = os.path.getsize

Datetime

I don't think this should be re-added to your class. If you feel it's of great importance then you should interface with os.stat_result. This means that using legacy and pathlib interfaces work with your changes.

Usage could be as simple as:

stats = DateTimeStat(os.stat(...))

Size Format

This shouldn't be on the class. If you want to format a number that is in bytes then define a function, not a method, to do this for you. This has the benefit of you being able to pass any number from any origin to the function and it will still happily work, DRYing up your code.

There are three common prefixes schemes when denoting binary sizes.

  • Metric (SI) prefixes - These are base 1000 and are the units you see pretty much everywhere in science. Much like I can have 1km, I have have 1kB. Which means 1000unit.
  • ISO binary prefix - There are base 1024 and are common when addressing binary data. The kibibyte, KiB, is 1024B.
  • JEDEC prefixes - These are discouraged by IEEE as they are confusing. It's not immediately apparent if you are using SI or JEDEC prefixes. These are base 1024 and only define the K, M, G prefixes.

I suggest you stick to SI with base 1000 and ISO with base 1024.
An example of all of them, 1Mib is 131.072kB and 128KB.

import functools


def size_format(size, base, units):
    for unit in units:
        if size < base:
            break
        size /= base
    return size, unit


size_bytes_format = functools.partial(
    size_format,
    base=1000,
    units='B kB MB GB TB YB'.split(),
)
size_ibytes_format = functools.partial(
    size_format,
    base=1024,
    units='B KiB MiB GiB TiB YiB'.split(),
)

Duplicating

I can't see this function being that useful. Even if I can't see a use, I wouldn't recommend adding it to the class. This is just a simple for loop. It's reasonable, and probably more readable, to defer this to the client code.

Result

Given all the above I would remove all the operations that act on an opened file. And it would result in the following class.

class Path(pathlib.Path):
    def unzip(self, destination=None):
        if destination is None:
            destination = self.parent
        with zipfile.ZipFile(self, 'r') as target:
            return target.extractall(destination)

    def copy(self, destination):
        return shutil.copy(self, destination)

File interactions

As stated earlier you shouldn't open the file in Path. I don't really see much benefit your class has over just using open. For example, it doesn't allow streaming and is pretty memory intense.

How is your code better than open and how is open better than your code?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.