jazzband / geojson

Python bindings and utilities for GeoJSON
https://pypi.python.org/pypi/geojson/
BSD 3-Clause "New" or "Revised" License
898 stars 120 forks source link

Points are not being validated correctly #143

Closed Rumpelstinsk closed 4 years ago

Rumpelstinsk commented 4 years ago

Check the code snippet. When you initialize a point using coordinates for a line, it says is a valid point:

import geojson

lineCoords=[[1,2],[3,4]]
pointCoords=[5,6]

print ("Line(lineCoords): ", geojson.LineString(lineCoords).is_valid)
print ("Line(pointCoors): ", geojson.LineString(pointCoords).is_valid)
print ("Point(pointCoords): ", geojson.Point(pointCoords).is_valid)
print ("Point(lineCoords): ", geojson.Point(lineCoords).is_valid)

Which produces the next exit:

Line(lineCoords): True Line(pointCoors): False Point(pointCoords): True Point(lineCoords): True

Initializing a Point using coordinates for a line should returns an invalid point. According to Apendix A.1 of RFC 7946

Point coordinates are in x, y order (easting, northing for projected coordinates, longitude, and latitude for geographic coordinates)

So, only an array of two numbers shold be considered as valid.

Rumpelstinsk commented 4 years ago

I have taken a look to geometry code. I think it can be resolved, with this change. Instead of:

def check_point(coord):
    if not isinstance(coord, list):
        return 'each position must be a list'
    if len(coord) not in (2, 3):
        return 'a position must have exactly 2 or 3 values'

Do:

import numbers

def check_point(coord):
    if not isinstance(coord, list):
        return 'each position must be a list'
    if len(coord) not in (2, 3):
        return 'a position must have exactly 2 or 3 values'
    for number in coord:
        if not isinstance(number, numbers.Number):
               return 'a position cannot have inner positions'

I assume the third coordinadate are for 3d. But RFC 7946 only supports 2D, So I think that sould be changed too. To this finall propose:

import numbers

def check_point(coord):
    if not isinstance(coord, list):
        return 'each position must be a list'
    if len(coord) != 2:
        return 'a position must have exactly 2 values'
    if not isinstance(coord[0], numbers.Number) or not isinstance(coord[1], numbers.Number):
         return 'a position cannot have inner positions'
auvipy commented 4 years ago

can you come with a PR?

Rumpelstinsk commented 4 years ago

@auvipy No problem. I have just made a pull request.

Rumpelstinsk commented 4 years ago

I saw the pull request was merged. I assume it will be available in the next release, so I close the issue