[Django] #18368: LayerMapping.save() should be able to set attributes on created model instances

Django

unread,

May 23, 2012, 12:57:20 PM5/23/12

to django-...@googlegroups.com

#18368: LayerMapping.save() should be able to set attributes on created model
instances
-----------------------------+--------------------
Reporter: geoffhing@… | Owner: nobody
Type: New feature | Status: new
Component: GIS | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------
As far as I can tell, {{{LayerMapping}}} only allows populating a spatial
model's attributes from the vector file. I recently had a use case where
I needed to set other attributes of the model instances based on
information not in the vector file.

I propose allowing an additional argument to {{{LayerMapping.save()}}}
that is a dictionary of initial field values that will be passed to the
constructor of the spatial model.

That is, changing the method signature to be:

{{{
def save(self, verbose=False, fid_range=False, step=False,
progress=False, silent=False, stream=sys.stdout,
strict=False,
model_kwargs=None):

}}}

and then inside {{{save()}}} around line 523, updating the keyword
arguments passed to the model constructor:

{{{
# Add additional keyword arguments
if model_kwargs:
kwargs.update(model_kwargs)

}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/18368>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

May 24, 2012, 4:05:05 AM5/24/12

to django-...@googlegroups.com

#18368: LayerMapping.save() should be able to set attributes on created model
instances

Easy pickings: 0 | UI/UX: 0

-----------------------------+--------------------------------------
Changes (by claudep):

* status: new => closed
* needs_better_patch: => 0
* resolution: => wontfix
* needs_tests: => 0
* needs_docs: => 0

Comment:

I'm not thrilled by this proposed addition. I don't see anything here that
cannot be done either by field default values on the model or by updating
the created objects in a second pass.

--
Ticket URL: <https://code.djangoproject.com/ticket/18368#comment:1>

Django

unread,

May 24, 2012, 11:10:08 AM5/24/12

to django-...@googlegroups.com

#18368: LayerMapping.save() should be able to set attributes on created model
instances

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-----------------------------+--------------------------------------
Changes (by geoffhing@…):

* status: closed => reopened
* resolution: wontfix =>

Comment:

@claudep, I just want to make sure you're clear on the use case that made
me request this. I'm bulk loading geometries using LayerMapping from a
management command (code at
https://github.com/PitonFoundation/atlas/blob/locations_places/apps/storybase_geo/management/commands/loadplaces.py).
The reason I needed to be able to pass additional model attributes to
LayerMapping.save() was to set a ForeignKey field to a model representing
levels of a geographic hierarchy. In my use case, this field can be null,
so that can't be used to identify the newly-created values. Also,
geometries may be loaded at different times and for different levels of
the geo hierarchy, so a default value for that field doesn't make sense.

Also, maybe I'm missing something, but I couldn't seem to find a way to
retrieve the model instances created by LayerMapping.save(). Certainly, I
could add a field to the model to indicate whether or not the model
instances had been post-processed in a second pass, but preferred not to
add fields to the model just to accommodate the occasional data load.

--
Ticket URL: <https://code.djangoproject.com/ticket/18368#comment:2>

Django

unread,

May 24, 2012, 2:15:04 PM5/24/12

to django-...@googlegroups.com

#18368: LayerMapping.save() should be able to set attributes on created model
instances

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-----------------------------+--------------------------------------

Comment (by claudep):

And what about post_save signal?

--
Ticket URL: <https://code.djangoproject.com/ticket/18368#comment:3>

Django

unread,

May 24, 2012, 3:06:50 PM5/24/12

to django-...@googlegroups.com

#18368: LayerMapping.save() should be able to set attributes on created model
instances

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-----------------------------+--------------------------------------
Changes (by geoffhing@…):

* status: reopened => closed
* resolution: => wontfix

Comment:

Replying to [comment:3 claudep]:

> And what about post_save signal?

You're absolutely right that signals are the way to go in this case.
However, I think it would have to be the pre_save signal in my case.
Thanks for the helpful observation.

--
Ticket URL: <https://code.djangoproject.com/ticket/18368#comment:4>

Django

unread,

Aug 10, 2017, 1:31:03 AM8/10/17

to django-...@googlegroups.com

#18368: LayerMapping.save() should be able to set attributes on created model
instances

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-----------------------------+--------------------------------------

Comment (by Austin Riba):

I still think LayerMapping.save() at the least should return the saved
object. I've run into the issue where I need to save data into the created
object not in the shapefile, like the original author. Unlike the OP
however a signal won't work (unless I'm missing something obvious) because
the context needs to come from the import script, it can't be done in
isolation.

--
Ticket URL: <https://code.djangoproject.com/ticket/18368#comment:5>

Reply all

Reply to author

Forward