Tuple vs List

[eseglem]

I understand the reasoning behind using a Tuple in the current implementation. It serializes out to valid GeoJSON, so its not wrong per se. But it does bug me to see it represented as a Tuple in code.

For Position something like this should fit the bill. As it allows for 2 or 3 elements.

Position = conlist(float, min_items=2, max_items=3)

BBox gets a bit trickier. You can use this:

BBox = Union[
    conlist(float, min_items=4, max_items=4),
    conlist(float, min_items=6, max_items=6)
]

However your errors end up confusing if you pass it 5 elements:

pydantic.error_wrappers.ValidationError: 2 validation errors for Foo
bbox
  ensure this value has at most 4 items (type=value_error.list.max_items; limit_value=4)
bbox
  ensure this value has at least 6 items (type=value_error.list.min_items; limit_value=6)

I was able to come up with this though:

from typing import Optional, List
from pydantic import BaseModel, ConstrainedList, PydanticValueError

class BBoxEvenItemsError(PydanticValueError):
    code = 'bbox.even_items'
    msg_template = 'ensure this value has an even number items'

class BBox(ConstrainedList):
    min_items = 4
    max_items = 6
    item_type = float
    __args__=(item_type,)

    @classmethod
    def list_length_validator(cls, v: Optional[List]) -> Optional[List]:
        v = super().list_length_validator(v)
        if v is None:
            return None
        if len(v) % 2 != 0:
            raise BBoxEvenItemsError()
        return v

Which is functionally conlist(float, min_items=4, max_items=6) plus a validator to make sure its also even.

[vincentsarago]

for reference this was already discussed in #42

[vincentsarago]

note that

BBox = Union[
    conlist(float, min_items=4, max_items=4),
    conlist(float, min_items=6, max_items=6)
]

Doesn't seems to be a valid expression for Mypy (python 3.8)

Invalid type alias: expression is not a valid type

[eseglem]

Yeah, unfortunately mypy won't execute functions so it doesn't acknowledge that it's a valid type but it does work on the pydantic side. You end up needing if TYPE_CHECKING and define it as a List[Float] if done that way.

I skimmed the other issue, and that was mostly about schema validation, correct? I believe the class should be ok. But I will do further checking on that front.

[vincentsarago]

Also not that in __geo_interface__ the spec seems to mention tuple instead of list

• https://gist.github.com/sgillies/2217756#__geo_interface__
• https://gist.github.com/sgillies/2217756?permalink_comment_id=1449897#gistcomment-1449897
• https://geopandas.org/en/stable/docs/reference/api/geopandas.GeoDataFrame.from_features.html
• https://github.com/shapely/shapely/blob/shapely-2.0/shapely/geometry/polygon.py#L87-L92

The Tuple Vs List seems to be a long running discussion. For now I'm not quite sure we should change it to List

[eseglem]

I think it would be valid to cast to tuple in the __geo_interface__ function. But I do understand more of the reasons now, and they make sense.

My complaint is mostly because of it visually not looking like geojson. You have to use () when writing it as code and [] when writing it as a string.

If I have coordinates with lists in the code pyright will complain that it's not the correct types. So you can't actually cut and paste geojson into your code as a dict without switching all the lists to tuples.

I think there might be tricks that could be done with an if TYPE_CHECKING to adjust it. I'll see if I can figure something out on that front.