[Django] #26333: makemigration on a PointField field with a default value does not use the value's class.
Django
value's class.
--------------------------+------------------------------------------------
Reporter: | Owner: nobody
simondrabble |
Type: Bug | Status: new
Component: | Version: 1.8
Migrations |
Severity: Normal | Keywords: makemigration gis point pointfield
Triage Stage: | Has patch: 0
Unreviewed |
Easy pickings: 0 | UI/UX: 0
--------------------------+------------------------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26333>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: simondrabble | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 1.8
Severity: Normal | Resolution:
Keywords: makemigration gis | Triage Stage: Accepted
point pointfield |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* needs_better_patch: => 0
* component: Migrations => GIS
* needs_tests: => 0
* needs_docs: => 0
* stage: Unreviewed => Accepted
Comment:
I think the `GEOSGeometry` class should be deconstructible.
Most of its subclasses are converted to `tuple`s by the
`IterableSerializer` as they define an `__iter__()` method.
--
Ticket URL: <https://code.djangoproject.com/ticket/26333#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: GIS | Version: 1.8
Severity: Normal | Resolution:
Keywords: makemigration gis | Triage Stage: Accepted
point pointfield |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by niconoe):
* status: new => assigned
* owner: nobody => niconoe
--
Ticket URL: <https://code.djangoproject.com/ticket/26333#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: simondrabble | Owner: niconoe
Type: Bug | Status: assigned
Component: GIS | Version: 1.8
Severity: Normal | Resolution:
Keywords: makemigration gis | Triage Stage: Accepted
point pointfield |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by niconoe):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/26333#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: simondrabble | Owner: niconoe
Type: Bug | Status: assigned
Component: GIS | Version: 1.8
Severity: Normal | Resolution:
Keywords: makemigration gis | Triage Stage: Accepted
point pointfield |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by charettes):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/26333#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: simondrabble | Owner: niconoe
Type: Bug | Status: assigned
Component: GIS | Version: 1.8
Severity: Normal | Resolution:
Keywords: makemigration gis | Triage Stage: Accepted
point pointfield |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by niconoe):
Indeed, I was using `CreateModel` in my test app. Thanks for your comment,
will do my best to solve this!
--
Ticket URL: <https://code.djangoproject.com/ticket/26333#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: simondrabble | Owner: niconoe
Type: Bug | Status: assigned
Component: GIS | Version: 1.8
Severity: Normal | Resolution:
Keywords: makemigration gis | Triage Stage: Accepted
point pointfield |
Needs tests: 0 | Patch needs improvement: 0
Changes (by niconoe):
* has_patch: 0 => 1
Comment:
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/26333#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: simondrabble | Owner: niconoe
Component: GIS | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: makemigration gis | Triage Stage: Accepted
point pointfield |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e158ec0ba0b43cbe9b509d4ed5587fa3b2ae3cc6" e158ec0b]:
{{{
#!CommitTicketReference repository=""
revision="e158ec0ba0b43cbe9b509d4ed5587fa3b2ae3cc6"
Fixed #26333 -- Made GIS Geometry classes deconstructible.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26333#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: simondrabble | Owner: niconoe
Type: Bug | Status: closed
Component: GIS | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: makemigration gis | Triage Stage: Accepted
point pointfield |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"d7334b405fb0e677e79cb064074df68188f0ddb5" d7334b4]:
{{{
#!CommitTicketReference repository=""
revision="d7334b405fb0e677e79cb064074df68188f0ddb5"
Refs #26333 -- Reverted inadvertent edits to fix tests.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26333#comment:8>