How to simplify function to find out if a string contains coordinates that describe polygon?

Question

I have the following string:

points = "34.09352 -118.27483, 34.0914 -118.2758, 34.082 -118.2782, 34.0937 -118.2769, 34.0933 -118.2748"

points is a string that contains coordinate values (latitude & longitude) separated by commas.

I want to check that this string contains only integers or float values and that the first coordinate equals the last.

I have the following code for that:

def validate_points(points):
   coordinates = points.split(',')

   for point in coordinates:
      latlon = point.split(' ')

      latitude = latlon[0]
      longitude = latlon[1]
      if not is_number(latitude) or not is_number(longitude):
         raise WrongRequestDataError("Please, specify the correct type of points value. It must be a numeric value")

   first = coordinates[0]
   last = coordinates[len(coordinates) - 1]
   if first != last:
        raise WrongRequestDataError("Incorrect points format, the first point must be equal to last")

def is_number(s):
   try:
     if float(s) or int(s):
        return True
   except ValueError:
        return False

Is there any way to simplify or speed up this code?

LGTM, you could write `coordinates[-1]` instead of `coordinates[len(coordinates)-1]`, though. — user2722968, Jul 19 '17 at 14:20
remember *if it can `int(..)` it can `float(..)` too*. So the `if float(s) or int(s)` can be reduced to `if float(s)` — Ma0, Jul 19 '17 at 14:36
FYI a polygon is not just a sequence of points. For example two points, expressed as `"Ax Ay, Bx By, Ax, Ay"` is not a polygon. And if any of the segments intersect then it is not a polygon. — Kirk Broadhurst, Jul 19 '17 at 14:37
@ipetr Note that your `is_number` function will return `None` for `s = '0'` for example. Because `not None` evaluates to `True`, strings like `'0'` are considered not to be numbers from your function. — a_guest, Jul 19 '17 at 14:57

Eric Duminil · Accepted Answer · 2017-07-19T14:50:21.683

Your input almost looks like a WKT polygon.

Using the shapely package, you could simply try to parse the points as WKT and see what happens, according to Python's "Easier to ask for forgiveness than permission" principle:

# pip install shapely
from shapely import wkt

def is_well_defined_polygon(points):
    try:
        wkt.loads("POLYGON((%s))" % points)
        return True
    except:
        return False

points = "34.09352 -118.27483, 34.0914 -118.2758, 34.082 -118.2782, 34.0937 -118.2769, 34.0933 -118.2748, 34.09352 -118.27483"

print(is_well_defined_polygon(points))
# True
print(is_well_defined_polygon("1 2, 3 4"))
# IllegalArgumentException: Points of LinearRing do not form a closed linestring
# False
print(is_well_defined_polygon("a b c d"))
# ParseException: Expected number but encountered word: 'a'
# False

you dropped the bomb on the this thread. It's not fun playing with you guys.. +1 xD — Ma0, Jul 19 '17 at 14:46

Cary Shindell · Answer 2 · 2017-07-19T14:33:24.337

Here are some improvements. You can speed up is_number function a bit, and use coordinates[-1] instead of `coordinates[len(coordinates)-1]. You also don't necessarily need to define all those variables:

def validate_points(points):
   coordinates = points.split(',')

   for point in coordinates:
       latitude, longitude = point.split(' ', 1)

       if not is_number(latitude) or not is_number(longitude):
          raise WrongRequestDataError("Please, specify the correct type of points value. It must be a numeric value")

   if coordinates[0] != coordinates[- 1]:
       raise WrongRequestDataError("Incorrect points format, the first point must be equal to last")
def is_number(s):
   try:
        return (float(s) or int(s) or True)
   except ValueError:
       return False

score 0 · Answer 3 · edited Jul 19 '17 at 16:40

0

Minor things:

Use coordinates[-1] instead of coordinates[len(coordinates)-1]
Use latitude, longitude = point.split(' ', 1). This will cause cases like 3.41 47.11 foobar to be invalid.

Do you really need latitude and longitude to be strings? You probably want the float/int value, so is_number should be something like

def conv_number(s):
    try:
        return float(s)
    except ValueError:
        try:
            return int(s)
        except ValueError:
            raise WrongRequestDataError(s)

I especially like the fact that you do not use isinstance to check for float/int: In python, you should always be able to pass an arbitrary object that acts like a int or float if asked to do so.

edited Jul 19 '17 at 16:40

Christian König

3,437
16
28

answered Jul 19 '17 at 14:28

user2722968

13,636
2
46
67

2

How is `isinstance` relevant for checking if a string is a valid number? Also we're not having "arbitrary" objects here but _strings_. Also I can't think of any case where `float` would raise but `int` doesn't (provided you don't change the `base` argument). – a_guest Jul 19 '17 at 14:33
people come up with `isinstance` all the time when dealing with type conversion. Prime example is taking an argument and `if not isinstance(foobar, str): raise TypeError` instead of just `str(foobar)`. – user2722968 Jul 19 '17 at 14:36
The question is not about type conversions. Also the question deals with the clearly defined API of the Python standard library, so I don't see how you expect something else to be returned from `str.split`. I.m.o. mentioning `isinstance` here is off the mark and just confuses the reader. Also could you clarify when you expect `float` to raise but similarly `int` not? – a_guest Jul 19 '17 at 14:45
You, sir, win one internets. – user2722968 Jul 19 '17 at 14:51

score 0 · Answer 4 · edited Jul 19 '17 at 14:50

0

That is how I would do it:

points = "34.09352 -118.27483, 34.0914 -118.2758, 34.082 -118.2782, 34.0937 -118.2769, 34.0933 -118.2748"


def validate_points(points):
    separate = points.split(',')

    try:
        [float(y) for x in separate for y in x.split()]
    except ValueError:
        return False

    return separate[0] == separate[-1]

print(validate_points(points))  # False

If you actually want to raise an Error you can modify\simplify the code as follows:

def validate_points(points):
    separate = points.split(',')
    [float(y) for x in separate for y in x.split()]  # orphan list-comprehension
    if not separate[0] == separate[-1]:
        raise ValueError

edited Jul 19 '17 at 14:50

a_guest

34,165
12
64
118

answered Jul 19 '17 at 14:32

Ma0

15,057
4
35
65

Thanks, but the algorithm doesn't need to return something. It's just throw an exception in case of invalid data – Jul 19 '17 at 14:35
What about `return False` in the `except` branch? This saves you the additional `else` branch and the `and` at the end. – a_guest Jul 19 '17 at 14:40

gsw945 · Answer 5 · 2017-07-19T15:34:31.150

my solution with using Regex named group to filter data:

# -*- coding: utf-8 -*-
import re


class WrongRequestDataError(Exception):
    pass

def position_equal(pos1, pos2):
    # retrun pos1 == pos2 # simple compare
    accuracy = 0.005
    return (
        abs(float(pos1['latitude']) - float(pos2['latitude'])) <= accuracy and
        abs(float(pos1['longitude']) - float(pos2['longitude'])) <= accuracy
    )

test_str = "34.09352 -118.27483, 34.0914 -118.2758, 34.082 -118.2782, 34.0937 -118.2769, 34.0933 -118.2748"

regex = r"(?P<position>(?P<latitude>\-?\d+(\.\d+)?) (?P<longitude>\-?\d+(\.\d+)?))"
matches = re.finditer(regex, test_str, re.IGNORECASE)

matched = []
for matchNum, match in enumerate(matches):
    matched.append({
        'latitude': match.group('latitude'),
        'longitude': match.group('longitude'),
    })

matched_count = len(matched)
if matched_count != test_str.count(',') + 1:
    raise WrongRequestDataError("Please, specify the correct type of points value. It must be a numeric value")
else:
    if matched_count > 1:
        if not position_equal(matched[0], matched[-1]):
            raise WrongRequestDataError("Incorrect points format, the first point must be equal to last")

You can modify the accuracy's value in position_equal function to change the accuracy when compare first and last position.

You can test or debug the regex at regex101: https://regex101.com/r/tYYJXN/1/

How to simplify function to find out if a string contains coordinates that describe polygon?

5 Answers5