3
\$\begingroup\$

I want to copy or delete a Path object, whether it is a file, a symlink, or a directory. I don't want to monkey patch Pathlib so I created some surrogate functions for those operations:

def copy(src: Path, target: Path):
    """ copy the file, symlink or path (empty or not)
    file/folder is replaced if already existing"""
    if src.is_file():
        shutil.copy(src, target)
    elif src.is_dir():
        shutil.copytree(src, target)

def delete(df: Path):
    """ delete the file, symlink or path (empty or not) """
    if df.is_dir() and not df.is_symlink():
        for dp, dn, fn in df.walk(top_down=False):
            for f in fn:
                (dp/f).unlink()
            dp.rmdir()
    if df.is_symlink() or df.is_file():
        df.unlink()

My goal is not to care about whether the Path exists or not. This verification will be carried out by the caller.

Do you see something missing or something that could be written in a more pythonic way? Should I handle some errors in it?

EDIT: One of the application for a dotfile manager would be:

def link_dotfile(df: Path):
    """ create a symlink from dotfile in the profile folder
    onto the home folder """
    home_dotfile = Path.home() / df.name
    if home_dotfile.exists():
        delete(home_dotfile)

    home_dotfile.symlink_to(df)

It removes the home dotfile which has the same name as the one stored in a third party folder and then creates a symlink pointing to it.

\$\endgroup\$
5
  • \$\begingroup\$ What is the purpose of these functions? If it's for exposing to a user who wants some simple file operations, then maybe it makes sense. If it's for some specific operation in a program then this is probably not well-advised. \$\endgroup\$
    – Reinderien
    Mar 8 at 12:57
  • \$\begingroup\$ Let's say those are shortcut for basic operations all along my program. For more fine operations I will for sure get back to the vanilla library methods. \$\endgroup\$
    – FTG
    Mar 8 at 13:10
  • 1
    \$\begingroup\$ all along my program definitely means that these are not well-applied functions. More often than not, within a program you'll know whether something is supposed to be a directory or not. \$\endgroup\$
    – Reinderien
    Mar 8 at 13:12
  • 1
    \$\begingroup\$ If you depict one or two places where these functions are called, I can offer more detail in an answer. \$\endgroup\$
    – Reinderien
    Mar 8 at 13:39
  • \$\begingroup\$ I have edited my post. \$\endgroup\$
    – FTG
    Mar 8 at 14:02

2 Answers 2

3
\$\begingroup\$

design of Public API

The Path routines were carefully designed to do just what they promise, without unpleasant surprises.

This API assumes that caller wants either of a file or a tree to be manipulated. Fine, I will assume that's what caller wants. The optional Path annotations in the signatures are very nice.

When documenting such an API, your docstrings should be pretty explicit about the fact that whole subtrees maybe acted on, causing e.g. rm -rf behavior. As written, I find the use of "path" in an English sentence to be a bit on the vague side. My initial interpretation of "empty path" was Path(""), clearly different from Author's Intent. OTOH a reference to "empty subdirectory" would have immediately illuminated the situation.

The {src, target} identifiers are very nice, thank you.

behavior differs from a familiar library

On first reading I imagined that delete() was roughly df.unlink() + shutil.rmtree(df), whichever was appropriate. But upon reading it I realized that there's no test suite which illustrates mkdir foo; date > foo/date.txt; ln -s foo bar; python -c '... delete("bar")'. As caller, I personally find it extremely surprising that bar got nuked (fine!), yet foo and its contents continue to exist. It seems pretty clear to me that a request to delete("bar") should obliterate the pointer and thing(s) pointed at. Oddly, the code won't do that.

It is perfectly fine to specify different behavior. But you should be pretty clear when you do that, drawing distinctions between the two libraries. Then potential callers will understand the advantages of your library for their particular use case.

meaningful identifiers

def delete(df: Path):  ...
        for dp, dn, fn in df.walk( ... ):

df is part of your Public API, and so has a heavier documentation burden than those three local variables. Please use a more descriptive name such as target or deleted_filespec.

The three locals are fine, but I would have appreciated root, directories, filenames, or simply copying dirpath, dirnames, filenames straight out of the man page. In particular, I got hung up on "singular, singular, singular" and was thinking we'd get one triple per file, when of course the underlying API actually returns "singular, plural_iterable, plural_iterable". Short names can be fine for locals, but hopefully they won't lead the Gentle Reader astray.

common lisp identifiers

                (dp/f).unlink()

Please don't write it in that manner. It looks like we're working with Scheme or with the Common Lisp variable dp/f, after executing (defvar dp/f "foo").

When writing python code we surround / slash and other operators with whitespace. Simplest way to accomplish that would be from the bash prompt: $ black *.py.

clear parameter names

def link_dotfile(df: Path):

Apparently here I should read it as "dotfile" pathname, rather than as "deleted file". Please spell it out: dotfile: Path.

Also, I found "onto" in the docstring confusing, in the sense that I failed to understand this function's contract. I thought we would be creating a new link to an existing dotfile. Turns out that we nuke a dotfile and then link out to a file within some pip-installed package subtree.

Consider verifying that df.exists() before doing anything drastic.

Arguably there's no need to verify that home_dotfile.exists(), since the delete() call will just be a no-op. Do consider tacking on a keyword default parameter of nonexistant_ok=True in its signature so callers will understand that aspect.

\$\endgroup\$
1
\$\begingroup\$

Let's pick on link_dotfile and assume that all other uses are similar in that they only operate on a file or a directory, not either. This is typical in a lot of applications. The give-away is in the name: link_dotfile links a file. It doesn't link a directory (or at least that behaviour has been obscured by nomenclature). If the names tell the truth, then

  • Delete copy and delete from your codebase; they supply an API choice that should not exist
  • Rewrite the likes of link_dotfile to
home_dotfile = Path.home() / df.name
home_dotfile.unlink(missing_ok=True)
home_dotfile.symlink_to(df)

If the names don't tell the truth and link_dotfile might truly link a directory, then name it something like link_dotpath.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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