geojson-pydantic types expect dicts and not json

[p1-dta]

The JSON validator of Pydantic expect a string json-formatted.

I understand why it's a good thing to validate python dict object with a valid geojson structure, but what if I already have a geojson string and I want to validate this string with the geojson-pydantic types?

Sure I can json.loads() before but I think it should be automated by the pydantic type.
Also, validate string is coherent with the pydantic.Json validator.

[vincentsarago]

@Vikka I'm not sure what you are trying to do, could you write down some pseudo code please?

pydantic comes with https://pydantic-docs.helpmanual.io/usage/models/#helper-functions which is maybe what you are looking for

from geojson_pydantic import Feature

f = '{"type": "Feature", "geometry": {"type": "Polygon", "coordinates": [[[13.38272, 52.46385], [13.42786, 52.46385], [13.42786, 52.48445], [13.38272, 52.48445], [13.38272, 52.46385]]]}}'

Feature.parse_raw(f)
Feature(type='Feature', geometry=Polygon(coordinates=[[(13.38272, 52.46385), (13.42786, 52.46385), (13.42786, 52.48445), (13.38272, 52.48445), (13.38272, 52.46385)]], type='Polygon'), properties=None, id=None, bbox=None)

[vincentsarago]

FYI I'm going to move this over the discussion because I don't think this is an issue

[p1-dta]

Hi,

I use Pydantic with FastAPI for data validation, and the geojson came from db return.

This is kind of thing I want do to:

@router.get("/endpoint/path")
def route_fct() -> list[geojson_pydantic.Polygon]:
    with get_session() as session:
        geojson_result = session.query(
            func.ST_AsGeoJSON(Model.geometry, options=2).label("geojson")
        ).first()
    return geojson_result[0].geojson

Comparison of Pydantic.Json and Polygon

from geojson_pydantic import Polygon
from pydantic import BaseModel, Json


class A(BaseModel):
    json_data: Json
    geojson: Polygon

json_str = '{"type": "Polygon","coordinates": [[[13.38272, 52.46385],[13.42786, 52.46385],[13.42786, 52.48445],[13.38272, 52.48445],[13.38272, 52.46385]]]}'
polygon = {
    "type": "Polygon",
    "coordinates": [
        [
            [13.38272, 52.46385],
            [13.42786, 52.46385],
            [13.42786, 52.48445],
            [13.38272, 52.48445],
            [13.38272, 52.46385],
        ]
    ],
}

print(A(json_data=json_str, geojson=polygon))
# json_data={'type': 'Polygon', 'coordinates': [[[13.38272, 52.46385], [13.42786, 52.46385], [13.42786, 52.48445], [13.38272, 52.48445], [13.38272, 52.46385]]]} 
# geojson=Polygon(coordinates=[[(13.38272, 52.46385), (13.42786, 52.46385), (13.42786, 52.48445), (13.38272, 52.48445), (13.38272, 52.46385)]], type='Polygon')

The result with fastAPI is the same, but the object we can validate isn't.
I understand why geojson_pydantic validate python object, but I think it would be a great adition if it can handle the same type as pydantic.Json (str, bytes or bytearray")

[vincentsarago]

I see. Pydantic Json is interesting but is described as a data type while geojson-pydantic are model and parsing is not something they're meant to. I don't think there is something to be done in geojson-pydantic. IMO it will be easier to deal with this at application level

[p1-dta]

Types used with pydantic also serve the role of parser. That's why pydantic.Json convert the type of the value.

That's why this works:

from pydantic import BaseModel


class A(BaseModel):
    some_integer: int

print(A(some_integer=1.5))
# some_integer=1

Types are used as data validators, and a validator convert the data into the wanted type, and if it fails, raise a ValidationError.
That's why validator should return the parsed value:

validators should either return the parsed value or raise a ValueError, TypeError, or AssertionError (assert statements may be used).

https://pydantic-docs.helpmanual.io/usage/validators/

In fact, even geojson types in pydantic validate object by parsing and return an other object, of the specified type.

A validator is ALSO a converter in pydantic, and since geojson is a json format, and json is string, it would be coherent if the validation of a specific json format validate string that "could" be valid geojson format.
So I really think that geojson validator should ALSO accepte string as input.

(PS: I'm not asking to change the returned value of the validator, just make the accepted type of geojson validators inputs from jsonable_object, to Union[jsonable_object, str])

[vincentsarago]

👍 I'll be happy to review any PR 🙏

[p1-dta]

Thanks, then I will do the PR in a few days.

[vincentsarago]

@Vikka should we also implement this for Feature and FeatureCollection ?

[p1-dta]

since it's in the rfc, IMO, yes.

[eseglem]

I have been reviewing a bunch of stuff in this package and came across the validate function which confused me.

I don't understand why any changes were necessary. The Json type in pydantic is a "Field" not an entire data Model. It is a special type of string, not a well defined type. Changing a Model to match its behavior seems strange.

If you just want to parse a string into an actual object Json already does that without modification. Json[Polygon]

class A(BaseModel):
    geojson: Json[Polygon]

print(A(geojson=json_str))
# geojson=Polygon(coordinates=[[(13.38272, 52.46385), (13.42786, 52.46385), (13.42786, 52.48445), (13.38272, 52.48445), (13.38272, 52.46385)]], type='Polygon')

Does that not accomplish the same thing?

[vincentsarago]

can one of you draft a PR to revert the change? 🙏

[p1-dta]

@eseglem
@vincentsarago
@geospatial-jeff

I find this solution elegant, but at a price: the result is not a str but it's considered as one.
This way, I can't do `my_polygon.coordinates' with autocompletion, it raise warnings. Yes, overriding of validate function seems complex, but it create user-friendly type.

I prefer complex (is it that complex?) code for obvious objects (understood by type checker and others) than simpler implementation that can mislead users.

So my opinion: keep previous implementation, override is fine to me. Even more because it's a public method.
Overriding validate function is a self validation, where @validator is member validation. And we create one each time we need one, and it's not a deal for anyone. Why self validation implementation is an issue?

So this is a small regression for me (behavior still the same, but now, typechecker complains).

[vincentsarago]

thanks @Vikka and sorry if I merged the PR too soon. I have absolutely 0 pro or cons so I guess the ball is on @eseglem side now!

[eseglem]

That is unfortunate. I did miss that type checking complains about it as I was just going through interactive. I see that pydantic is just defining it as a str for type checking rather than a fancy Generic. But the old way I saw complaints from type checker because a string isn't a valid input for Polygon either.

I don't have a problem with overriding validate in general, my problem is with overriding it to behave differently than expected. The previous implementation changed many things:

• It not only added an unexpected behavior (parsing a string) it removed the expected behavior (the rest of validate). The .parse_raw function exists and is separate from the default validate function for a reason. As a user of a something based on Pydantic I would never expect it to do string parsing (of json) by default. It is confusing to change that behavior and certainly not misleading to leave it as the default.
• It also removes the ability to more strictly control input. If a spec says something needs to be geojson, and not a stringified version of it, but some API user sends a string, this function will let that right though. The only way I can think of to prevent that would be to make a new class and override the validate function back to default.
• It hard codes the use of json.loads so the Config.json_loads setting may also be ignored. Probably not a huge deal in this context, but its worth being aware of.

I definitely feel its best to leave it as default pydantic behavior and let anyone who wants a different behavior to change it themselves.

And I use geojson-pydantic for moving GeoJSON in and out of Postgres so I understand the use case well. I just use a @validate function if I need to switch something up. And I have a custom geoalchemy2.Geometry based class that does most of the casting in and out of the DB side. So I have fine grained control of how the data is being manipluated.

I do have an idea that might be an OK compromise. This is untested, but something like:

ALLOW_FROM_STRING = False  # or some similar name

class Foo(BaseModel):
    def validate(cls, value):
        if ALLOW_FROM_STRING and isinstance(str, value):
            return cls.parse_raw(value)
        return super().validate(value)

Then importing it and setting geojson_pydantic.ALLOW_FROM_STRING = True I think would allow it at the library level. I don't feel its that necessary of a feature but if there is demand for it, I think it might still function without changing the default pydantic behavior. I also don't know much about the implications of doing that in a library are though. Seems like it could be problematic in some cases.

[p1-dta]

thanks @Vikka and sorry if I merged the PR too soon. I have absolutely 0 pro or cons so I guess the ball is on @eseglem side now!

No worries :)

@eseglem

I agree 100%.

For the suggestion, I disagree and don't think it's a thing we should do.
Yes t's problematics as ALLOW_FROM_STRING is global to the module, which behave as a singleton. High risk of bugs for users of this feature unaware of this property. To solve my issue,

I should have used .parse_raw, period, but I didn't know this method at that time and misunderstood a lot of things.

So, I'm now against my changes and pro default behaviour 😄

Thank you for your time, all of you!

[p1-dta]

Just use .parse_raw() or Json[Polygon] return type if it's ok to return a string.

See discussion here: #66 (comment)