bottlepy / bottle

bottle.py is a fast and simple micro-framework for python web-applications.
http://bottlepy.org/
MIT License
8.33k stars 1.46k forks source link

Accept pathlib paths across the board #1311

Open Jwink3101 opened 3 years ago

Jwink3101 commented 3 years ago

This is an issue in preparation for a patch I will be trying to make.

I want to update Bottle to accept pathlib.Path paths across the board. My plan is to do this in a backwards COMPATIBLE way by adding something like str(inputfile) so that if you pass a native string or unicode, nothing happens but if you pass a pathlib object, it gets converted.

First question: Does this plan make sense? I use this in my tools so that I don't have to think about what is passed and it is explicitly converted

Second question: Where should I be looking? I think the goal should be as much coverage as possible but I would take better over perfect*. Off the top of my head:

Anywhere else?

Other thoughts?

*It took the python standard library a few major versions to transition too.

oz123 commented 3 years ago

Can you post an example snippet of how this change would look?

defnull commented 3 years ago

Sounds fine, but instead of str() I'd suggest to add a (private) _strpath() function that raises TypeError for value types that make no sense. Using str() would render sanity checks in other libraries useless, as for example None would be passed on as a string.

Jwink3101 commented 3 years ago

re Private method

It makes sense to use a private _strpath() method, but there are some complexities in that. The main issue is that (for some reason) pathlib does not offer its own method to turn a path into a string. The method suggested method in the docs is to use str(). So you cannot look for a certain attribute to call to make it a string. So without directly importing it, you can't do isinstance

This is also further complicated by the fact that (as I found in trying to answer oz123's question below), there are instance checks for base-string. So instead I propose the following. Note that this is NOT done in the if py3k conditional to account for backports

try:
    from pathlib import Path # 3.4+ or backport in python2
except ImportError:
    class Path(object):
        pass # dummy class. We never call it. Only used for instance checks
...

def _strpath(path):
    """Convert pathlib.Path objects"""
    if isinstance(path,Path):
        return unicode(path)
    return path 

(I use unicode to again support pyhton2)

Of course, the other option is to import pathlib and just do an instance check but that would break python2. I've seen in other PRs that you are hesitant to break python2 "just because". This is a reason to but it is also pretty minor. And then option 2.5 which is leave this open and don't make changes until we're already broken python2

re example:

Here is an example, but it actually shows the complexity. This is in the FileUpload class

    def save(self, destination, overwrite=False, chunk_size=2 ** 16):
        """ Save file to disk or copy its content to an open file(-like) object.
            If *destination* is a directory, :attr:`filename` is added to the
            path. Existing files are not overwritten by default (IOError).

            :param destination: File path, directory, pathlib.Path, or file(-like) object.
            :param overwrite: If True, replace existing files. (default: False)
            :param chunk_size: Bytes to read at a time. (default: 64kb)
        """
        if isinstance(destination, (basestring,Path)):  # Except file-likes here
            destination = _strpath(destination)
            if os.path.isdir(destination):
                destination = os.path.join(destination, self.filename)
            if not overwrite and os.path.exists(destination):
                raise IOError('File exists.')
            with open(destination, 'wb') as fp:
                self._copy_file(fp, chunk_size)
        else:
            self._copy_file(destination, chunk_size)

So not too bad I guess.

I just need to find all of the examples where this happens.

Marrin commented 3 years ago

Shoudn't be the instance check against os.PathLike and/or make a test against the __fspath__ attribute.

Jwink3101 commented 3 years ago

That's new in 3.6. So it won't support backported pathlib (which I am fine with) and older python.

What is the minimum Bottle should support? And I again ask, is this the issue to break support? Probably not.

iamgodot commented 2 years ago

Is this still in progress? I'm trying to achieve the same thing, also maybe something like str(path.resolve()) would do better.