From 212d3876dca9c124790e7a1aac30708991e4181d Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Mon, 8 May 2017 13:13:00 +0100 Subject: [PATCH 1/3] [UK] Use database for co-ordinate transformation. If the application is running on a server with GDAL 1.9 (and maybe 1.8), its OSGB co-ordinate transformation can be up to a few metres out. As we use the transformation by the database when saving new coordinates, also use it when fetching so we can match and display accurate information. https://trac.osgeo.org/gdal/ticket/4597 is the GDAL ticket that explains that it is fixed in GDAL 1.10. --- .../commands/mapit_import_postal_codes.py | 13 ++++------ mapit/models.py | 17 ++++++++----- mapit_gb/countries.py | 13 ++++------ mapit_gb/tests.py | 24 ++++++++++++------- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/mapit/management/commands/mapit_import_postal_codes.py b/mapit/management/commands/mapit_import_postal_codes.py index f6b62cfe..ae092b1f 100644 --- a/mapit/management/commands/mapit_import_postal_codes.py +++ b/mapit/management/commands/mapit_import_postal_codes.py @@ -131,17 +131,12 @@ def do_postcode(self, location=None, srid=None): pc = Postcode.objects.get(postcode=self.code) if location: if pc.location: - curr_location = (pc.location[0], pc.location[1]) + curr_location = pc.location + # Postcode locations are stored as WGS84 if settings.MAPIT_COUNTRY == 'GB': - if pc.postcode[0:2] == 'BT': - curr_location = pc.as_irish_grid() - else: - pc.location.transform(27700) # Postcode locations are stored as WGS84 - curr_location = (pc.location[0], pc.location[1]) - curr_location = tuple(map(round, curr_location)) + curr_location = pc.as_uk_grid() elif srid != 4326: - pc.location.transform(srid) # Postcode locations are stored as WGS84 - curr_location = (pc.location[0], pc.location[1]) + curr_location = pc.location.transform(srid, clone=True) if curr_location[0] != location[0] or curr_location[1] != location[1]: pc.location = location pc.save() diff --git a/mapit/models.py b/mapit/models.py index c882c48b..a9173d1e 100644 --- a/mapit/models.py +++ b/mapit/models.py @@ -413,6 +413,10 @@ def filter_by_area(self, area): ) +def str2int(s): + return int(round(float(s))) + + @python_2_unicode_compatible class Postcode(models.Model): postcode = models.CharField(max_length=7, db_index=True, unique=True) @@ -449,12 +453,13 @@ def as_dict(self): countries.augment_postcode(self, result) return result - # Doing this via self.location.transform(29902) gives incorrect results. - # The database has the right proj4 text, the proj file does not. I think. - def as_irish_grid(self): + # Doing this via self.location.transform(27700/29902) can give incorrect results + # with some versions of GDAL. Via the database produces a correct result. + def as_uk_grid(self): cursor = connection.cursor() - cursor.execute("SELECT ST_AsText(ST_Transform(ST_GeomFromText('POINT(%f %f)', 4326), 29902))" % ( - self.location[0], self.location[1])) + srid = 29902 if self.postcode[0:2] == 'BT' else 27700 + cursor.execute("SELECT ST_AsText(ST_Transform(ST_GeomFromText('POINT(%f %f)', 4326), %d))" % ( + self.location[0], self.location[1], srid)) row = cursor.fetchone() m = re.match('POINT\((.*?) (.*)\)', row[0]) - return list(map(float, m.groups())) + return list(map(str2int, m.groups())) diff --git a/mapit_gb/countries.py b/mapit_gb/countries.py index 57669124..c279290b 100644 --- a/mapit_gb/countries.py +++ b/mapit_gb/countries.py @@ -81,15 +81,10 @@ def augment_postcode(postcode, result): pc = postcode.postcode if is_special_postcode(pc): return - if pc[0:2] == 'BT': - loc = postcode.as_irish_grid() - result['coordsyst'] = 'I' - else: - loc = postcode.location - loc.transform(27700) - result['coordsyst'] = 'G' - result['easting'] = int(round(loc[0])) - result['northing'] = int(round(loc[1])) + loc = postcode.as_uk_grid() + result['coordsyst'] = 'I' if pc[0:2] == 'BT' else 'G' + result['easting'] = loc[0] + result['northing'] = loc[1] # Hacky function to restrict certain geographical links in the HTML pages to diff --git a/mapit_gb/tests.py b/mapit_gb/tests.py index 0f650d36..0e891030 100644 --- a/mapit_gb/tests.py +++ b/mapit_gb/tests.py @@ -67,21 +67,27 @@ def setUp(self): code='E05000025', ) + def test_ni_postcode(self): + postcode = models.Postcode(postcode='BT170XD', location=Point(-6.037555, 54.556533)) + grid = postcode.as_uk_grid() + self.assertEqual(grid, [327011, 369351]) + def test_postcode_json(self): pc = self.postcode.postcode url = '/postcode/%s' % urllib.parse.quote(pc) response = self.client.get(url) content = get_content(response) - in_gb_coords = self.postcode.location.transform(27700, clone=True) + in_gb_coords = self.postcode.as_uk_grid() + self.assertEqual(in_gb_coords, [529090, 179645]) pc = countries.get_postcode_display(self.postcode.postcode) self.assertDictEqual(content, { 'postcode': pc, 'wgs84_lat': self.postcode.location.y, 'wgs84_lon': self.postcode.location.x, 'coordsyst': 'G', - 'easting': round(in_gb_coords.x), - 'northing': round(in_gb_coords.y), + 'easting': round(in_gb_coords[0]), + 'northing': round(in_gb_coords[1]), 'areas': { str(self.area.id): { 'id': self.area.id, @@ -113,14 +119,14 @@ def test_partial_json(self): response = self.client.get(url) content = get_content(response) countries.get_postcode_display(self.postcode.postcode) - in_gb_coords = self.postcode.location.transform(27700, clone=True) + in_gb_coords = self.postcode.as_uk_grid() self.assertDictEqual(content, { 'wgs84_lat': self.postcode.location.y, 'wgs84_lon': self.postcode.location.x, 'coordsyst': 'G', 'postcode': 'SW1A', - 'easting': round(in_gb_coords.x), - 'northing': round(in_gb_coords.y) + 'easting': round(in_gb_coords[0]), + 'northing': round(in_gb_coords[1]) }) def test_partial_json_link(self): @@ -133,7 +139,7 @@ def test_nearest_json(self): response = self.client.get(url) content = get_content(response) pc = countries.get_postcode_display(self.postcode.postcode) - in_gb_coords = self.postcode.location.transform(27700, clone=True) + in_gb_coords = self.postcode.as_uk_grid() self.assertDictEqual(content, { 'postcode': { 'distance': 0, @@ -141,8 +147,8 @@ def test_nearest_json(self): 'wgs84_lon': self.postcode.location.x, 'coordsyst': 'G', 'postcode': pc, - 'easting': round(in_gb_coords.x), - 'northing': round(in_gb_coords.y) + 'easting': round(in_gb_coords[0]), + 'northing': round(in_gb_coords[1]) } }) From 2b47eaed542e88dcaf40fa11c8f39f3c125b135b Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Tue, 9 May 2017 13:36:59 +0100 Subject: [PATCH 2/3] Improve test running with changed settings/order. A couple of tests were e.g. altering mapit.models.countries and not setting it back, and another two would only pass if COUNTRY was set to 'GB'. Set those to skip, and restore original settings. --- mapit/tests/test_names.py | 4 ++++ mapit/tests/test_views.py | 7 +++---- mapit_gb/tests.py | 9 +++++++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/mapit/tests/test_names.py b/mapit/tests/test_names.py index 146b38ef..9c1d87c8 100644 --- a/mapit/tests/test_names.py +++ b/mapit/tests/test_names.py @@ -41,14 +41,18 @@ def setUp(self): def test_new_name_changes_area_name_in_gb(self): """We can't use override_settings, as mapit.countries has been set based upon MAPIT_COUNTRY already in initial import""" + orig_countries = mapit.models.countries mapit.models.countries = mapit_gb.countries Name.objects.create(name='New Name (B)', type=self.name_type, area=self.area) self.assertEqual(self.area.name, 'New Name Borough') + mapit.models.countries = orig_countries def test_new_name_does_not_change_area_name_elsewhere(self): + orig_countries = mapit.models.countries mapit.models.countries = None Name.objects.create(name='New Name', type=self.name_type, area=self.area) self.assertEqual(self.area.name, 'Big Area') + mapit.models.countries = orig_countries def test_geometry_name_works(self): name = smart_text('Big “Area”') diff --git a/mapit/tests/test_views.py b/mapit/tests/test_views.py index 7ea42667..bcb75446 100644 --- a/mapit/tests/test_views.py +++ b/mapit/tests/test_views.py @@ -1,4 +1,5 @@ import json +import unittest from django.test import TestCase from django.conf import settings @@ -6,8 +7,6 @@ from mapit.models import Type, Area, Geometry, Generation, Postcode from mapit.tests.utils import get_content -import mapit_gb.countries -import mapit.views class AreaViewsTest(TestCase): @@ -89,8 +88,8 @@ def test_front_page(self): response = self.client.get('/') self.assertContains(response, 'MapIt') + @unittest.skipUnless(settings.MAPIT_COUNTRY == 'GB', 'UK only test') def test_postcode_submission(self): - mapit.views.countries = mapit_gb.countries response = self.client.post('/postcode/', {'pc': 'PO14 1NT'}, follow=True) self.assertRedirects(response, '/postcode/PO141NT.html') response = self.client.post('/postcode/', {'pc': 'PO141NT.'}, follow=True) @@ -107,7 +106,7 @@ def test_example_postcode(self): url = '/area/%d/example_postcode' % id response = self.client.get(url) content = get_content(response) - self.assertEqual(content, self.postcode.postcode) + self.assertEqual(content, str(self.postcode)) def test_nearest_with_bad_srid(self): url = '/nearest/84/0,0.json' diff --git a/mapit_gb/tests.py b/mapit_gb/tests.py index 0e891030..98007f95 100644 --- a/mapit_gb/tests.py +++ b/mapit_gb/tests.py @@ -1,3 +1,4 @@ +import unittest from django.conf import settings from django.test import TestCase from django.contrib.gis.geos import Polygon, Point @@ -16,8 +17,8 @@ def url_postcode(pc): class GBViewsTest(TestCase): def setUp(self): # Make sure we're using the GB postcode functions here - models.countries = countries - utils.countries = countries + self.orig_countries = models.countries + models.countries = utils.countries = countries self.postcode = models.Postcode.objects.create( postcode='SW1A1AA', @@ -67,6 +68,9 @@ def setUp(self): code='E05000025', ) + def tearDown(self): + models.countries = utils.countries = self.orig_countries + def test_ni_postcode(self): postcode = models.Postcode(postcode='BT170XD', location=Point(-6.037555, 54.556533)) grid = postcode.as_uk_grid() @@ -158,6 +162,7 @@ def test_nearest_json_link(self): pc = self.postcode.postcode self.assertContains(response, '"/postcode/%s"' % url_postcode(pc)) + @unittest.skipUnless(settings.MAPIT_COUNTRY == 'GB', 'UK only tests') def test_gss_code(self): response = self.client.get('/area/E05000025') self.assertRedirects(response, '/area/%d' % self.area.id) From 130a8d8973fe8a64891beb81129456be65ea9cd6 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Tue, 9 May 2017 14:34:23 +0100 Subject: [PATCH 3/3] Use 6 decimal places on postcode HTML page. No idea why the latitude in the wmflabs link was being limited to 1 decimal place previously! --- mapit/templates/mapit/postcode.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mapit/templates/mapit/postcode.html b/mapit/templates/mapit/postcode.html index 7369216d..e798aaa4 100644 --- a/mapit/templates/mapit/postcode.html +++ b/mapit/templates/mapit/postcode.html @@ -13,7 +13,7 @@

{{ postcode.postcode }}

E/N: {{ postcode.easting }}, {{ postcode.northing }} {% endif %} {% if postcode.wgs84_lat or postcode.wgs84_lon %} -
  • {% trans "WGS84 lat/lon" %}: {{ postcode.wgs84_lat }}, {{ postcode.wgs84_lon }} +
  • {% trans "WGS84 lat/lon" %}: {{ postcode.wgs84_lat|floatformat:-6 }}, {{ postcode.wgs84_lon|floatformat:-6 }} {% else %}
  • {% blocktrans trimmed %} No location information. Note this is a valid postcode (otherwise you would have got a 404), just one for which we don’t know the location. @@ -36,7 +36,7 @@

    {{ postcode.postcode }}

    maxZoom: 18 }); - var point = new L.LatLng({{ postcode.wgs84_lat }}, {{ postcode.wgs84_lon }}); + var point = new L.LatLng({{ postcode.wgs84_lat|floatformat:-6 }}, {{ postcode.wgs84_lon|floatformat:-6 }}); var marker = new L.Marker(point); map.addLayer(marker); map.setView(point, 14);