Create management command for importing Post shapes #249
Conversation
|
Hi @jeancochrane, I'm sorry we should have written down what we thought the plan was after our conversation yesterday. I want the |
|
Shouldn't be too difficult of a change I think, thanks for clarifying @fgregg! |
jeancochrane
changed the title
jeancochrane
changed the title
| model_name='person', | ||
| name='headshot', | ||
| field=models.FileField(default='images/headshot_placeholder.png', storage=django.core.files.storage.FileSystemStorage(base_url='/', location='/Users/goobzie/datamade/chi-councilmatic'), upload_to='images/headshots'), | ||
| ), |
There was a problem hiding this comment.
Seems like this headshot migration didn't get caught earlier. It also has a path specific to my machine, since the storage param is set dynamically. If it'll pose a problem, I can remove this from the migration and we can deal with it in a separate PR.
There was a problem hiding this comment.
this does not seem like what we want. Let's deal with headshots in a separate PR.
There was a problem hiding this comment.
Got it, I removed this operation from the migration.
councilmatic_core/models.py
Outdated
| @@ -243,6 +247,8 @@ class Meta: | |||
| on_delete=models.CASCADE, | |||
| ) | |||
|
|
|||
| shape = JSONField() | |||
There was a problem hiding this comment.
In the 0.x line, Post.shape was stored as a TextField and data was de/serialized by hand. I'm assuming this is because 0.x was written before Django and Postgres added full support for JSONField, so I went ahead and adjusted it.
There was a problem hiding this comment.
I don't know if it's better, but the database is postgis enabled if there would be any facilities we wanted to take advantage of there.
There was a problem hiding this comment.
I don't think it would make any of our current application logic easier, but it does seem like a better abstraction to store it as a GeometryField, and we'd get some amount of database-level validation to boot. I'll go ahead and change the storage format.
councilmatic_core/models.py
Outdated
| @@ -243,6 +247,8 @@ class Meta: | |||
| on_delete=models.CASCADE, | |||
| ) | |||
|
|
|||
| shape = JSONField() | |||
There was a problem hiding this comment.
I don't know if it's better, but the database is postgis enabled if there would be any facilities we wanted to take advantage of there.
| model_name='person', | ||
| name='headshot', | ||
| field=models.FileField(default='images/headshot_placeholder.png', storage=django.core.files.storage.FileSystemStorage(base_url='/', location='/Users/goobzie/datamade/chi-councilmatic'), upload_to='images/headshots'), | ||
| ), |
There was a problem hiding this comment.
this does not seem like what we want. Let's deal with headshots in a separate PR.
| 'shape_file', | ||
| help=( | ||
| 'The location of the file containing shapes for each Division, ' | ||
| 'relative to the project root. The file should be formatted as JSON ' |
There was a problem hiding this comment.
From an authoring perspective, I think it would be easier to get or make a geojson file and then edit the properties to add division ids, so the geojson file would look something like:
{
"type": "FeatureCollection",
"features": [
{
"type": "Feature",
"geometry": {
"type": "Point",
"coordinates": [102.0, 0.5]
},
"properties": {
"division_id": "/some/intersting/unique_id/division_id"
}
},
...otherwise, we'll need to get some shapefile and transform it into this special format geojson file?
There was a problem hiding this comment.
That seems reasonable to me. This custom JSON object was the format that minimized the amount of transformation we'd have to do above and beyond what existed in import_data already, but I agree with you that GeoJSON is probably an easier import format for end users to deal with. Changed this up so that it expects GeoJSON as input, and adjusted chi-councilmatic accordingly.
|
Reflecting a little on this on my train ride home. Post is not really the
perfect place to put the borders of a division. However, right now within
the OCD spec, there's not really a better place to put it.
This issue discusses the right approach (imo), but has not been implemented
yet. opencivicdata/python-opencivicdata#110
If and when it does, we can revisit this issue. No changes required now.
…On Wed, Jun 26, 2019 at 5:06 PM Jean Cochrane ***@***.***> wrote:
@jeancochrane <https://github.com/jeancochrane> requested your review on:
#249 <#249> Create
management command for importing Post shapes.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#249?email_source=notifications&email_token=AAEDC3J2P6GUCQL5U4Q33RTP4PR7FA5CNFSM4H3LUDP2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOSGJUE6A#event-2442347128>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEDC3KNSTNKXHELJS7742TP4PR7FANCNFSM4H3LUDPQ>
.
|
Overview
With the OCD model migration in #240, we stripped out the
import_datascript that imported boundaries into the app. Create animport_shapescommand that will pull these boundaries from from an OCD API and associate them with their correspondingPost.Closes #248.
Notes
When opening #248, we'd discussed trying to refactor boundaries to be served as static files instead of being stored in the
Post.shapefield. Ultimately we determined that would be a bigger refactor than we had scope for, so we went with the approach you see here, which simply refactors the old management command logic for the new models.Testing instructions
Test these changes by following the instructions in
chi-councilmatic: datamade/chi-councilmatic#256