Summary

Bump djblets django dependency to 1.6

Review Request #5309 — Created Jan. 23, 2014 and submitted Feb. 3, 2014, 12:56 p.m.

Information

Owner

david

Repository

Djblets

Branch

master

Bugs

Depends On

Reviewers

Groups

djblets

People

Description

Bump djblets django dependency to 1.6

This is relatively straightforward. The changes here include: * Bumping django to >= 1.6.1 * Replacing PIL with Pillow as the preferred image library. Existing PIL
installations will continue to work. * Remove obsolete call to setup_environ() in build-media.py * Fix call to compile_messages() in build-i18n.py * Changing some tests in the datagrid to account for the fact that 'timesince'
now uses non-breaking spaces between the number and the unit.

Testing Done


Ran unit tests.
Installed djblets and saw that it used PIL if installed, and preferred Pillow

  if not.

Issues

Description	From	Last Updated
This is going to cause all kinds of problems for upgrades, according to the docs. We'd also have to document …	chipx86	Jan. 31, 2014, 4:08 p.m.
I don't think this is guaranteed to work, due to how PIL is packaged in some cases, hence the import …	chipx86	Jan. 31, 2014, 5:15 p.m.
Can we keep this inline, and just use a variable for the PIL vs. Pillow dependency?	chipx86	Jan. 31, 2014, 10:59 p.m.

setup.py (Diff revision 1)

The issue has been resolved. Show all issues

This is going to cause all kinds of problems for upgrades, according to the docs. We'd also have to document how to install it and all prereqs, since a lot of our supported installs are less likely to package Pillow.
Since PIL support is going away in 1.8, and is compatible until then, let's change this to allow for both. This will give people time to move away from PIL without breaking people.
We should also then emit a deprecation warning in rb-site, and add some docs.
Googling around shows that this pattern is common: https://github.com/mapproxy/mapproxy/blob/master/setup.py
I'd suggest we inverse that, though we'd probably have to do it with import testing, given that PIL isn't very setuptools-friendly. So, require PIL if it's there and Pillow is not, and otherwise require Pillow.

chipx86 Jan. 31, 2014, 2:47 p.m.

Maybe a better base for the PIL part of the check: https://github.com/frol/python-cropresize2/blob/master/setup.py

Change Summary:


Update PIL vs. Pillow to be more forgiving.
Make changes to build-* helpers.

Description:

		Bump djblets django dependency to 1.6

		This is relatively straightforward. The changes here include:
		* Bumping django to >= 1.6.1
~		* Replacing PIL with Pillow
	~	* Replacing PIL with Pillow as the preferred image library. Existing PIL
	+	installations will continue to work.
	+	* Remove obsolete call to `setup_environ()` in build-media.py
	+	* Fix call to `compile_messages()` in build-i18n.py
		* Changing some tests in the datagrid to account for the fact that 'timesince'
		now uses non-breaking spaces between the number and the unit.

Testing Done:

~		Ran unit tests.
	~	Ran unit tests.
	+	Installed djblets and saw that it used PIL if installed, and preferred Pillow if not.

Diff:

Revision 2 (+35 -15)

Show changes

	setup.py
	contrib/internal/build-i18n.py
	contrib/internal/build-media.py
	djblets/datagrid/tests.py

setup.py (Diff revision 2)

The issue has been dropped. Show all issues

I don't think this is guaranteed to work, due to how PIL is packaged in some cases, hence the import approach.

david Jan. 31, 2014, 5:06 p.m.

I don't understand how this is different from using require(). Either pkg_resources knows about "PIL" or it doesn't.

david Jan. 31, 2014, 5:09 p.m.

And because Pillow and PIL deliberately have the same imports, we can't just import "Image" to see if it works.

chipx86 Jan. 31, 2014, 5:15 p.m.

My concern was with the way it registers in that sometimes you have a "PIL" module and sometimes you have an "Image" module.

I'm probably wrong. Since we require it as a dependency, it must be fulfilled on existing installs.

david Jan. 31, 2014, 5:16 p.m.

The "PIL" import was long deprecated, even when the "PIL" library was maintained.

setup.py (Diff revision 2)

The issue has been resolved. Show all issues

Can we keep this inline, and just use a variable for the PIL vs. Pillow dependency?

Diff:

Revision 3 (+29 -10)

Show changes

	setup.py
	contrib/internal/build-i18n.py
	contrib/internal/build-media.py
	djblets/datagrid/tests.py

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to master (4b23988).