Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#26333 closed Bug (fixed)

GIS geometries classes should be deconstructibles.

Reported by: simondrabble Owned by: Nicolas Noé
Component: GIS Version: 1.8
Severity: Normal Keywords: makemigration gis point pointfield
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Given:

    from django.contrib.gis.db import models as gis
    from django.contrib.gis.geos import Point
    from django.db import models
    
    POINT = Point(-104.9903, 39.7392, srid=4326)
    
    class PagedModel(models.Model):
        objects = gis.GeoManager()
        location = gis.PointField(srid=4326, default=POINT)

then

python manage.py makemigration
python manage.py migrate

Observed:

  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/core/management/__init__.py", line 354, in execute_from_command_line
    utility.execute()
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/core/management/__init__.py", line 346, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/core/management/base.py", line 394, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/core/management/base.py", line 445, in execute
    output = self.handle(*args, **options)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 222, in handle
    executor.migrate(targets, plan, fake=fake, fake_initial=fake_initial)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/db/migrations/executor.py", line 110, in migrate
    self.apply_migration(states[migration], migration, fake=fake, fake_initial=fake_initial)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/db/migrations/executor.py", line 148, in apply_migration
    state = migration.apply(state, schema_editor)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/db/migrations/migration.py", line 115, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/db/migrations/operations/fields.py", line 62, in database_forwards
    field,
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/contrib/gis/db/backends/postgis/schema.py", line 94, in add_field
    super(PostGISSchemaEditor, self).add_field(model, field)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 384, in add_field
    definition, params = self.column_sql(model, field, include_default=True)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/contrib/gis/db/backends/postgis/schema.py", line 30, in column_sql
    column_sql = super(PostGISSchemaEditor, self).column_sql(model, field, include_default)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 146, in column_sql
    default_value = self.effective_default(field)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 211, in effective_default
    default = field.get_db_prep_save(default, self.connection)
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/contrib/gis/db/models/fields.py", line 307, in get_db_prep_save
    return connection.ops.Adapter(self.get_prep_value(value))
  File "/pyenv/django1.8/lib/python2.7/site-packages/django/contrib/gis/db/models/fields.py", line 210, in get_prep_value
    raise ValueError('Cannot use object with type %s for a geometry lookup parameter.' % type(geom).__name__)
ValueError: Cannot use object with type float for a geometry lookup parameter.

Expected:

Migration completes successfully.

Hand-editing the generate migration file to use the Point class results in success.

IOW, changing:

            field=django.contrib.gis.db.models.fields.PointField(default=(-104.9903, 39.7392), srid=4326),

to

            field=django.contrib.gis.db.models.fields.PointField(default=Point(-104.9903, 39.7392, srid=4326)),

resolves the issue.

Change History (8)

comment:1 Changed 6 years ago by Simon Charette

Component: MigrationsGIS
Summary: makemigration on a PointField field with a default value does not use the value's class.GIS geometries classes should be deconstructibles.
Triage Stage: UnreviewedAccepted

I think the GEOSGeometry class should be deconstructible.

Most of its subclasses are converted to tuples by the IterableSerializer as they define an __iter__() method.

comment:2 Changed 6 years ago by Nicolas Noé

Owner: changed from nobody to Nicolas Noé
Status: newassigned

comment:3 Changed 6 years ago by Nicolas Noé

Hi,

I just realized I'm not able to reproduce the initial issue. I've tested against 1.8, 1.9.5 and current version.

Should we close the issue? Or continue working to make sure GEOSGeometry is deconstructible as suggested by charettes, if that has other benefits?

comment:4 Changed 6 years ago by Simon Charette

I can still reproduce against the master branch.

The Point is still converted to a tuple of floats (which simply drops the srid) and migrate crash if the field's default is actually used (e.g. AddField instead of CreateModel where it's simply ignored).

Have a look at this test app for more details: https://github.com/charettes/django-ticketing/commit/dac11b52249e7a061fea81944a5f11e2b92cee86

comment:5 Changed 6 years ago by Nicolas Noé

Indeed, I was using CreateModel in my test app. Thanks for your comment, will do my best to solve this!

comment:6 Changed 6 years ago by Nicolas Noé

Has patch: set

Just created a pull request for this, testing various geometries with various constructors.

Should I also write more functional tests to make sure the initial issue is solved (and that migration actually works with Point), or is testing that those are deconstructible enough?

comment:7 Changed 6 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In e158ec0b:

Fixed #26333 -- Made GIS Geometry classes deconstructible.

comment:8 Changed 6 years ago by Tim Graham <timograham@…>

In d7334b4:

Refs #26333 -- Reverted inadvertent edits to fix tests.

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