Opened 7 years ago

Closed 4 years ago

#28428 closed New feature (fixed)

Add support for Pathlib objects in django.core.files.storage

Reported by: Tom Forbes Owned by: nobody
Component: File uploads/storage Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Code in django.core.storage will explode when given a Pathlib object on Python 3.4 and 3.5. For example, this throws a cryptic AttributeError: 'PosixPath' object has no attribute 'rfind' exception due to generate_filename only expecting strings:

def upload_to(filename):
      return pathlib.Path(filename)

class Storage(models.Model):
   file = models.FileField(upload_to=upload_to)

More generally, it would be nice if all methods in the django.core.files accepted pathlib objects, and used pathlib objects internally. For compatibility reasons I think strings would have to be returned.

Change History (13)

comment:1 by Tim Graham, 7 years ago

Component: UncategorizedFile uploads/storage

Do you have a use case in mind? What are the advantages?

comment:2 by Tom Forbes, 7 years ago

In my case we have a bunch of methods in our applications that all use pathlib instances, some of which is used by our upload_to methods. It feels a bit strange to have to return the string representation from them when interacting with Django. With Python 2 support out of the picture there isn't much reason to, other than historical ones.

The main advantage is readability, in a lot of cases using Pathlib makes os.path heavy code a lot less dense.

comment:3 by Tim Graham, 7 years ago

Triage Stage: UnreviewedSomeday/Maybe

I'm not sure. I guess I'd have to see a patch to evaluate the idea.

comment:4 by Roger G. Coram, 7 years ago

As of Python 3.6 (specifically running 3.6.3) django.core.files.storage.Storage.generate_filename doesn't appear to raise this error when passed a pathlib.Path. Instead the os.path.split therein appears to handle it as expected:

Python 3.6.3 (default, Oct  6 2017, 08:44:35) 
>>> import os, pathlib
>>> os.path.split(pathlib.PosixPath("some/folder/test_with_space.txt"))
('some/folder', 'test_with_space.txt')

comment:5 by Kyle Agronick, 6 years ago

Pathlib provides a much nicer object oriented syntax rather than dealing with strings. os.path supports pathlib. Django really should too. At the very least all you would have to do is cast to a string and everything else would work as normal.

comment:6 by Claude Paroz, 5 years ago

Has patch: set
Triage Stage: Someday/MaybeAccepted

PR.

It's easier now that Python 3.6 is the minimal version.

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In af69842d:

Refs #28428 -- Added test for a callable FileField.upload_to that returns pathlib.Path.

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In d1c2e6dd:

Refs #28428 -- Made FileField.upload_to support pathlib.Path.

comment:9 by Mariusz Felisiak, 5 years ago

Has patch: unset

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 6315a27:

Refs #28428 -- Made filepath_to_uri() support pathlib.Path.

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 0f297553:

Refs #28428 -- Added tests for using FileSystemStorage with pathlib.Path.

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 17752003:

Refs #28428 -- Made FileSystemStorage.save() to support pathlib.Path.

comment:13 by Mariusz Felisiak, 4 years ago

Resolution: fixed
Status: newclosed
Summary: Add support for Pathlib objects in django.core.storageAdd support for Pathlib objects in django.core.files.storage

All FileSystemStorage's methods now support pathlib.Path().

Note: See TracTickets for help on using tickets.
Back to Top