Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PlaceHolderImage Image size 0 #13

Closed
hagsteel opened this issue May 14, 2015 · 4 comments
Closed

PlaceHolderImage Image size 0 #13

hagsteel opened this issue May 14, 2015 · 4 comments

Comments

@hagsteel
Copy link

OnDiscPlaceholderImage seem to have a bug in it:

This line: self.image_data = ContentFile(file.read(), name=name) causes the file to be read.
The next line super(OnDiscPlaceholderImage, self).__init__(file, name) is setting self.image_data again using file.read(), however file.read() will now return '' thus self.image_data is always ''

I found this when I was writing a custom placeholder image class that reads from STATIC rather than MEDIA

@respondcreate
Copy link
Owner

Thanks for filing this ticket, @jonashagstedt! I'm investigating now and will drop you a message here when I've got it sorted.

respondcreate added a commit that referenced this issue May 15, 2015
Includes:
* Updating tests
* Swapping out placeholder.gif for placeholder.png (since PIL doesn’t
know how to invert ‘P’ images:
https://github.com/python-pillow/Pillow/blob/master/PIL/ImageOps.py#L50)
@respondcreate
Copy link
Owner

Hey @jonashagstedt !

This issue has been patched and the fix is available in the 1.0.1 release. Upgrade via pip and you should be good to go:

$ pip install django-versatileimagefield==1.0.1

Thanks again for filing this ticket!

@hagsteel
Copy link
Author

Smashing.
I really like this package (it's a great idea!).
One thing that irked me, which is why I wrote my own PlaceholderImageClass (and I've seen this with SORL as well) is that placeholders comes from MEDIA rather than STATIC, and MEDIA is usually not retained between releases / dev machine whereas STATIC files are.

I might give this a go again though now that you've fixed it.

Cheers

@respondcreate
Copy link
Owner

Hey @jonashagstedt!

Thanks for the kind words about the app!

The idea for OnDiscPlaceholderImage came directly from the desire to use same placeholder image across different deployment environments without having to constantly rsync media across them. Previously I had loaded my placeholders from STATIC but started to run into SuspiciousOperation issues when trying to size/store different renditions of them in either a STATIC_DIR or STATIC_ROOT directory.

Once I realized that an on-disc image could be copied onto a field's storage class during startup (and thereby served as MEDIA) it was clear this was the 'best of both worlds' solution I had been searching for.

Don't hesitate to file another issue if you run into any more bugs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants