Fixed #23804 -- Added RasterField for PostGIS #4339
Conversation
|
If it helps, we might consider dropping support for PostGIS 1.5 as its initial release was Feb. 2010. |
|
Rasters were only added to PostGIS in version 2.0, so any raster handling in previous versions would have to be through a regular binary field or so and would not support any spatial querying that could be implemented down the line for proper raster fields. So for rasters, dropping support for PostGIS 1.5 would make sense. |
|
PR to drop PostGIS 1.5 support: #4345 |
|
Claude believes we should keep PostGIS 1.5 support for 1.9, so you'll need to skip these tests there. |
|
Sounds good to me. Is there an example somewhere in the codebase for a db version check? |
|
Yes, you can look at my proposed PR to drop PostGIS 1.5 or grep for "ops.spatial_version". By the way, I did email the geodjango mailing list to ask for objections to dropping PostGIS 1.5 support in 1.9, so maybe that decision isn't final. |
yellowcap
force-pushed
the
raster_field
branch
7 times, most recently
from
95c9d6d
to
e301eb9
Compare
|
dropping support for postgis less then 2.0 in 1.9 is fine |
yellowcap
force-pushed
the
raster_field
branch
4 times, most recently
from
26f877c
to
b7fca21
Compare
|
Is there a way of limiting the import of test models based on the db backend? The build keeps failing for I already wrapped the model defininition in a |
| from django.db import connection | ||
|
|
||
| HAS_POSTGIS_2 = ( | ||
| hasattr(connection.ops, 'postgis') and |
There was a problem hiding this comment.
is there a reason for using vendor checks instead of the supports_raster feature flag?
There was a problem hiding this comment.
There was one, but not anymore. I updated the test to use the feature flag and moved the import of the test model into the test. That solved the problem. Thanks!
| """ | ||
|
|
||
| description = "Raster Field" | ||
| geom_type = 'RASTER' |
There was a problem hiding this comment.
This is not ideal naming, but it leverages the geom_type attribute checks in the operations module, so renaming this to be more general (to include raster) would require updating the name in the geometry related code as well.
yellowcap
force-pushed
the
raster_field
branch
5 times, most recently
from
1d5edac
to
0e075bc
Compare
| self.assertIn('path', indexes) | ||
| self.assertSpatialIndexExists('gis_neighborhood', 'path') | ||
|
|
||
| @skipUnlessDBFeature("supports_raster") |
There was a problem hiding this comment.
try to be consistent and use single quotes in most places (sorry existing code is inconsistent)
yellowcap
force-pushed
the
raster_field
branch
2 times, most recently
from
eb6b59c
to
b395e8f
Compare
|
Time to add docs? |
yellowcap
force-pushed
the
raster_field
branch
3 times, most recently
from
1c5a600
to
db9afdd
Compare
|
I added documentation for the RasterField. I hope I found all the relevant bits, I was not sure how to make clear that this is only supported for PostGIS backends, so I added it in one or two places where I found it relevant. It would probably be good to also integrate the raster field into the GeoDjango Tutorial, but maybe that is another pull request? |
|
Thanks! I'll let Tim review the language. |
yellowcap
force-pushed
the
raster_field
branch
2 times, most recently
from
4544ce5
to
d069462
Compare
|
Minor edits: http://dpaste.com/1YSTF0M and add release notes too? |
|
Integrated comments and added release notes. |
|
please squash commits, thanks |
Thanks to Tim Graham and Claude Paroz for the reviews and patches.
|
Corrected those glitches and squashed commits. |
timgraham
changed the title
|
merged in b769bbd, thanks! |
|
Awesome, congrats Daniel! |
|
Feels good! A big thanks you both for all the help! |
|
Sorry, I forgot to try running the tests on systems without the GIS dependencies installed. Daniel, can you take a look? https://code.djangoproject.com/ticket/25011 |
|
Here is a patch for it #4905 I think the same problem will also happen on systems that have only geos installed, so I think the |
Refs #23804.