🚧 Some improvements for Coordinate type PR

[lig]

No description provided.

[JeanArhancet]

Thanks @lig

On my side I've change your implementation to this (parse_args -> parse_str -> parse_tuple) but latitude, longitude validation no longer works. WDYT?

@classmethod
    def __get_pydantic_core_schema__(
            cls, source: Type[Any], handler: GetCoreSchemaHandler
    ) -> core_schema.CoreSchema:
        float_schema = core_schema.float_schema()
        result = core_schema.no_info_before_validator_function(
            cls._parse_args,
            core_schema.union_schema(
                [
                    handler(source),
                    core_schema.no_info_before_validator_function(
                        cls._parse_str,
                        core_schema.no_info_after_validator_function(
                            cls._parse_tuple,
                            core_schema.tuple_positional_schema([float_schema, float_schema]),
                        ),
                    )
                ]
            ),
        )
        return result

    @classmethod
    def _parse_args(cls, value: Any) -> Any:
        if not isinstance(value, ArgsKwargs):
            return value

        args, kwargs = value.args, value.kwargs

        if kwargs:
            return kwargs
        if not args:
            return cls._NULL_ISLAND
        if len(args) == 1:
            return args[0]
        return args

    @classmethod
    def _parse_tuple(cls, value: tuple[float, float]) -> 'Coordinate':
        return cls(latitude=Latitude(value[0]), longitude=Longitude(value[1]))

    @classmethod
    def _parse_str(cls, value: Any) -> Tuple[float, float]:
        if isinstance(value, (str, bytes)):
            try:
                value = tuple([float(x) for x in value.split(',')])
            except ValueError:
                raise PydanticCustomError(
                    'coordinate_error',
                    'value is not a valid coordinate: string is not recognized as a valid coordinate'
                )

        return value

[JeanArhancet]

On my branch I've modified this part to add a default latitude, otherwise we'll get an error

[lig]

This is an error about the error case that is needed. We get an error for case like (20.0,) that is a tuple with a single value. This case is in tests as invalid.

[JeanArhancet]

You’re right but maybe we can raise an error directly, WDYT?

[JeanArhancet]

I've updated the function __get_pydantic_core_schema_ from the commit 687f74b and the properties does refer to the Longitude/Latitude limit.
WDYT @lig ?

    def __get_pydantic_core_schema__(cls, source: type[Any], handler: GetCoreSchemaHandler) -> core_schema.CoreSchema:
        return core_schema.no_info_wrap_validator_function(
            cls._parse_args,
            core_schema.no_info_wrap_validator_function(
                cls._parse_str,
                core_schema.chain_schema(
                    [
                        core_schema.no_info_wrap_validator_function(
                            cls._parse_tuple, handler.generate_schema(Tuple[Latitude, Longitude])
                        ),
                        handler(source),
                    ]
                ),
            ),
        )

Schema:

 {
                'properties': {
                    'x': {
                        'format': 'coordinate',
                        'maxItems': 2,
                        'minItems': 2,
                        'prefixItems': [
                            {'maximum': 90.0, 'minimum': -90.0, 'type': 'number'},
                            {'maximum': 180.0, 'minimum': -180.0, 'type': 'number'},
                        ],
                        'title': 'X',
                        'type': 'array',
                    }
                },
                'required': ['x'],
                'title': 'Model',
                'type': 'object',
            },
}

[JeanArhancet]

Thanks for your help, but I don't understand why range(3) and schema_chain[2 - x :] ?

[lig]

The full schema chain includes all three steps of conversion.
We allow entering this chain of transformations from any step. Thus we generate something that could be simplified like this: union([chain([3]), chain([2, 3]), chain([1, 2, 3])]).

[lig]

@JeanArhancet Sorry, I was ill. Now I'm back:)

I've updated my changes. The idea here is that we accept three forms of coordinate for validation and using a canonical form for serialization. This is reflected in the schema being generated for this type (see test_json_schema test)

In my opinion representing Coordinate as object by default for serialization is more useful for further processing data by external consumers, e.g. SPA Web App.

It's possible to implement another type, i.e. CoordinateStr, that uses string representation for serialization. This shouldn't necessarily be a part of this PR.

Please, notice that schema generation requires pydantic>=2.0.3 because of pydantic/pydantic#6515

[JeanArhancet]

@JeanArhancet Sorry, I was ill. Now I'm back:)

I've updated my changes. The idea here is that we accept three forms of coordinate for validation and using a canonical form for serialization. This is reflected in the schema being generated for this type (see test_json_schema test)

In my opinion representing Coordinate as object by default for serialization is more useful for further processing data by external consumers, e.g. SPA Web App.

It's possible to implement another type, i.e. CoordinateStr, that uses string representation for serialization. This shouldn't necessarily be a part of this PR.

Please, notice that schema generation requires pydantic>=2.0.3 because of pydantic/pydantic#6515

I understand better thanks a lot. Maybe we can create a variable to explain this 3 ? numSchemaSupported ?(or comments), WDYT?

[lig]

I understand better thanks a lot. Maybe we can create a variable to explain this 3 ? numSchemaSupported ?(or comments), WDYT?

Indeed, it could be more readable and robust to do something like this: chain_length = len(schema_chain) and use this variable.

Could you please take it from here on your PR?