Commit a79e6b67 authored by Claude Paroz's avatar Claude Paroz
Browse files

Fixed #24152 -- Deprecated GeoQuerySet aggregate methods 

Thanks Josh Smeaton and Tim Graham for the reviews.
parent 5338ff48

django/contrib/gis/db/models/aggregates.py

+4 −4
Original line number Diff line number Diff line 
from django.db.models.aggregates import Aggregate

from django.contrib.gis.db.models.fields import GeometryField, ExtentField

from django.contrib.gis.db.models.fields import ExtentField



__all__ = ['Collect', 'Extent', 'Extent3D', 'MakeLine', 'Union']
@@ -20,9 +20,9 @@ class GeoAggregate(Aggregate): 
            self.template = '%(function)s(SDOAGGRTYPE(%(expressions)s,%(tolerance)s))'

        return self.as_sql(compiler, connection)



    def prepare(self, query=None, allow_joins=True, reuse=None, summarize=False):

        c = super(GeoAggregate, self).prepare(query, allow_joins, reuse, summarize)

        if not isinstance(self.expressions[0].output_field, GeometryField):

    def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False):

        c = super(GeoAggregate, self).resolve_expression(query, allow_joins, reuse, summarize, for_save)

        if not hasattr(c.input_field.field, 'geom_type'):

            raise ValueError('Geospatial aggregates only allowed on geometry fields.')

        return c

django/contrib/gis/db/models/query.py

+28 −0
Original line number Diff line number Diff line 
import warnings



from django.db import connections

from django.db.models.expressions import RawSQL

from django.db.models.fields import Field
@@ -15,6 +17,7 @@ from django.contrib.gis.geometry.backend import Geometry 
from django.contrib.gis.measure import Area, Distance



from django.utils import six

from django.utils.deprecation import RemovedInDjango20Warning





class GeoQuerySet(QuerySet):
@@ -65,6 +68,11 @@ class GeoQuerySet(QuerySet): 
        This is analogous to a union operation, but much faster because

        boundaries are not dissolved.

        """

        warnings.warn(

            "The collect GeoQuerySet method is deprecated. Use the Collect() "

            "aggregate in an aggregate() or annotate() method.",

            RemovedInDjango20Warning, stacklevel=2

        )

        return self._spatial_aggregate(aggregates.Collect, **kwargs)



    def difference(self, geom, **kwargs):
@@ -105,6 +113,11 @@ class GeoQuerySet(QuerySet): 
        Returns the extent (aggregate) of the features in the GeoQuerySet.  The

        extent will be returned as a 4-tuple, consisting of (xmin, ymin, xmax, ymax).

        """

        warnings.warn(

            "The extent GeoQuerySet method is deprecated. Use the Extent() "

            "aggregate in an aggregate() or annotate() method.",

            RemovedInDjango20Warning, stacklevel=2

        )

        return self._spatial_aggregate(aggregates.Extent, **kwargs)



    def extent3d(self, **kwargs):
@@ -113,6 +126,11 @@ class GeoQuerySet(QuerySet): 
        GeoQuerySet. It is returned as a 6-tuple, comprising:

          (xmin, ymin, zmin, xmax, ymax, zmax).

        """

        warnings.warn(

            "The extent3d GeoQuerySet method is deprecated. Use the Extent3D() "

            "aggregate in an aggregate() or annotate() method.",

            RemovedInDjango20Warning, stacklevel=2

        )

        return self._spatial_aggregate(aggregates.Extent3D, **kwargs)



    def force_rhr(self, **kwargs):
@@ -215,6 +233,11 @@ class GeoQuerySet(QuerySet): 
        this GeoQuerySet and returns it.  This is a spatial aggregate

        method, and thus returns a geometry rather than a GeoQuerySet.

        """

        warnings.warn(

            "The make_line GeoQuerySet method is deprecated. Use the MakeLine() "

            "aggregate in an aggregate() or annotate() method.",

            RemovedInDjango20Warning, stacklevel=2

        )

        return self._spatial_aggregate(aggregates.MakeLine, geo_field_type=PointField, **kwargs)



    def mem_size(self, **kwargs):
@@ -398,6 +421,11 @@ class GeoQuerySet(QuerySet): 
        None if the GeoQuerySet is empty.  The `tolerance` keyword is for

        Oracle backends only.

        """

        warnings.warn(

            "The unionagg GeoQuerySet method is deprecated. Use the Union() "

            "aggregate in an aggregate() or annotate() method.",

            RemovedInDjango20Warning, stacklevel=2

        )

        return self._spatial_aggregate(aggregates.Union, **kwargs)



    ### Private API -- Abstracted DRY routines. ###

django/contrib/gis/tests/geo3d/tests.py

+4 −1
Original line number Diff line number Diff line
@@ -6,7 +6,8 @@ from unittest import skipUnless 


from django.contrib.gis.gdal import HAS_GDAL

from django.contrib.gis.geos import HAS_GEOS

from django.test import TestCase, skipUnlessDBFeature

from django.test import TestCase, ignore_warnings, skipUnlessDBFeature

from django.utils.deprecation import RemovedInDjango20Warning

from django.utils._os import upath



if HAS_GEOS:
@@ -206,6 +207,7 @@ class Geo3DTest(TestCase): 
        # Ordering of points in the resulting geometry may vary between implementations

        self.assertSetEqual({p.ewkt for p in ref_union}, {p.ewkt for p in union})



    @ignore_warnings(category=RemovedInDjango20Warning)

    def test_extent(self):

        """

        Testing the Extent3D aggregate for 3D models.
@@ -223,6 +225,7 @@ class Geo3DTest(TestCase): 
        for e3d in [extent1, extent2]:

            check_extent3d(e3d)

        self.assertIsNone(City3D.objects.none().extent3d())

        self.assertIsNone(City3D.objects.none().aggregate(Extent3D('point'))['point__extent3d'])



    def test_perimeter(self):

        """

django/contrib/gis/tests/geoapp/test_regress.py

+2 −1
Original line number Diff line number Diff line
@@ -3,6 +3,7 @@ from __future__ import unicode_literals 


from datetime import datetime



from django.contrib.gis.db.models import Extent

from django.contrib.gis.geos import HAS_GEOS

from django.contrib.gis.shortcuts import render_to_kmz

from django.contrib.gis.tests.utils import no_oracle
@@ -44,7 +45,7 @@ class GeoRegressionTests(TestCase): 
        "Testing `extent` on a table with a single point. See #11827."

        pnt = City.objects.get(name='Pueblo').point

        ref_ext = (pnt.x, pnt.y, pnt.x, pnt.y)

        extent = City.objects.filter(name='Pueblo').extent()

        extent = City.objects.filter(name='Pueblo').aggregate(Extent('point'))['point__extent']

        for ref_val, val in zip(ref_ext, extent):

            self.assertAlmostEqual(ref_val, val, 4)

django/contrib/gis/tests/geoapp/tests.py

+37 −10
Original line number Diff line number Diff line
@@ -5,11 +5,13 @@ from tempfile import NamedTemporaryFile 


from django.db import connection

from django.contrib.gis import gdal

from django.contrib.gis.db.models import Extent, MakeLine, Union

from django.contrib.gis.geos import HAS_GEOS

from django.contrib.gis.tests.utils import no_oracle, oracle, postgis, spatialite

from django.core.management import call_command

from django.test import TestCase, skipUnlessDBFeature

from django.test import TestCase, ignore_warnings, skipUnlessDBFeature

from django.utils import six

from django.utils.deprecation import RemovedInDjango20Warning



if HAS_GEOS:

    from django.contrib.gis.geos import (fromstr, GEOSGeometry,
@@ -470,19 +472,26 @@ class GeoQuerySetTest(TestCase): 
            self.assertIsInstance(country.envelope, Polygon)



    @skipUnlessDBFeature("supports_extent_aggr")

    @ignore_warnings(category=RemovedInDjango20Warning)

    def test_extent(self):

        "Testing the `extent` GeoQuerySet method."

        """

        Testing the (deprecated) `extent` GeoQuerySet method and the Extent

        aggregate.

        """

        # Reference query:

        # `SELECT ST_extent(point) FROM geoapp_city WHERE (name='Houston' or name='Dallas');`

        #   =>  BOX(-96.8016128540039 29.7633724212646,-95.3631439208984 32.7820587158203)

        expected = (-96.8016128540039, 29.7633724212646, -95.3631439208984, 32.782058715820)



        qs = City.objects.filter(name__in=('Houston', 'Dallas'))

        extent = qs.extent()

        extent1 = qs.extent()

        extent2 = qs.aggregate(Extent('point'))['point__extent']



        for extent in (extent1, extent2):

            for val, exp in zip(extent, expected):

                self.assertAlmostEqual(exp, val, 4)

        self.assertIsNone(City.objects.filter(name=('Smalltown')).extent())

        self.assertIsNone(City.objects.filter(name=('Smalltown')).aggregate(Extent('point'))['point__extent'])



    @skipUnlessDBFeature("has_force_rhr_method")

    def test_force_rhr(self):
@@ -614,11 +623,17 @@ class GeoQuerySetTest(TestCase): 


    # Only PostGIS has support for the MakeLine aggregate.

    @skipUnlessDBFeature("supports_make_line_aggr")

    @ignore_warnings(category=RemovedInDjango20Warning)

    def test_make_line(self):

        "Testing the `make_line` GeoQuerySet method."

        """

        Testing the (deprecated) `make_line` GeoQuerySet method and the MakeLine

        aggregate.

        """

        # Ensuring that a `TypeError` is raised on models without PointFields.

        self.assertRaises(TypeError, State.objects.make_line)

        self.assertRaises(TypeError, Country.objects.make_line)

        # MakeLine on an inappropriate field returns simply None

        self.assertIsNone(State.objects.aggregate(MakeLine('poly'))['poly__makeline'])

        # Reference query:

        # SELECT AsText(ST_MakeLine(geoapp_city.point)) FROM geoapp_city;

        ref_line = GEOSGeometry(
@@ -629,7 +644,9 @@ class GeoQuerySetTest(TestCase): 
        )

        # We check for equality with a tolerance of 10e-5 which is a lower bound

        # of the precisions of ref_line coordinates

        line = City.objects.make_line()

        line1 = City.objects.make_line()

        line2 = City.objects.aggregate(MakeLine('point'))['point__makeline']

        for line in (line1, line2):

            self.assertTrue(ref_line.equals_exact(line, tolerance=10e-5),

                "%s != %s" % (ref_line, line))
@@ -813,24 +830,34 @@ class GeoQuerySetTest(TestCase): 
    # but this seems unexpected and should be investigated to determine the cause.

    @skipUnlessDBFeature("has_unionagg_method")

    @no_oracle

    @ignore_warnings(category=RemovedInDjango20Warning)

    def test_unionagg(self):

        "Testing the `unionagg` (aggregate union) GeoQuerySet method."

        """

        Testing the (deprecated) `unionagg` (aggregate union) GeoQuerySet method

        and the Union aggregate.

        """

        tx = Country.objects.get(name='Texas').mpoly

        # Houston, Dallas -- Ordering may differ depending on backend or GEOS version.

        union1 = fromstr('MULTIPOINT(-96.801611 32.782057,-95.363151 29.763374)')

        union2 = fromstr('MULTIPOINT(-95.363151 29.763374,-96.801611 32.782057)')

        qs = City.objects.filter(point__within=tx)

        self.assertRaises(TypeError, qs.unionagg, 'name')

        self.assertRaises(ValueError, qs.aggregate, Union('name'))

        # Using `field_name` keyword argument in one query and specifying an

        # order in the other (which should not be used because this is

        # an aggregate method on a spatial column)

        u1 = qs.unionagg(field_name='point')

        u2 = qs.order_by('name').unionagg()

        u3 = qs.aggregate(Union('point'))['point__union']

        u4 = qs.order_by('name').aggregate(Union('point'))['point__union']

        tol = 0.00001

        self.assertTrue(union1.equals_exact(u1, tol) or union2.equals_exact(u1, tol))

        self.assertTrue(union1.equals_exact(u2, tol) or union2.equals_exact(u2, tol))

        self.assertTrue(union1.equals_exact(u3, tol) or union2.equals_exact(u3, tol))

        self.assertTrue(union1.equals_exact(u4, tol) or union2.equals_exact(u4, tol))

        qs = City.objects.filter(name='NotACity')

        self.assertIsNone(qs.unionagg(field_name='point'))

        self.assertIsNone(qs.aggregate(Union('point'))['point__union'])



    def test_non_concrete_field(self):

        NonConcreteModel.objects.create(point=Point(0, 0), name='name')