Custom conversions

[LukeMathWalker]

This is related to #6, after having spent some time using the library in a bunch of production services.
"Default" type conversions are fine most of the times (with some exception - see #9), but there are several occasions in which you'd like to override the conversion function for a field.
This is usually dates, where the format has to be spelled out, but it could be as well classes with configuration parameters that you might want to capture into a lambda function using partial application.

As of today, what I usually do is writing a wrapper function that first takes care of the fields requiring custom conversion and then relies on undictify to deal with the remaining fields.
Would you consider adding an option in type_checked_constructor to specify a dictionary of field_name -> conversion function to override the default behaviour?

[Dobiasd]

The concept sounds OK to me.

Currently the signatures look as follows:

def type_checked_constructor(skip: bool = False,
                             convert: bool = False) -> Callable[[Callable[..., TypeT]],
                                                                Callable[..., TypeT]]:

def type_checked_call(skip: bool = False,
                      convert: bool = False) -> Callable[[Callable[..., TypeT]],
                                                         Callable[..., TypeT]]:

I'll need to check what to do with the type annotations.
Dict[str, Callable[[Any], Any]] currently looks so untyped to me. ;)

[LukeMathWalker]

Unfortunately the expressiveness of type annotations in Python only goes up to a certain point 👅
It's difficult to capture this thing into a sufficiently general but meaningful type signature, I agree.

[Dobiasd]

OK, to make sure I understand correctly. You have challenges similar to this:

from datetime import datetime
import json
from undictify import type_checked_constructor
from typing import NamedTuple, Union

@type_checked_constructor()
class Foo(NamedTuple):
    msg: str
    timestamp: datetime

json_str = """{
    "msg": "hi",
    "timestamp": "Jun 1 2005  1:33PM"
}"""

foo = Foo(**json.loads(json_str))
print(foo.msg)
print(foo.timestamp)

And currently you are solving them like that:

from datetime import datetime
import json
from undictify import type_checked_constructor
from typing import NamedTuple, Union

class MyDatetime:
    def __init__(self, repr: str) -> None:
        self.val = datetime.strptime(repr, '%b %d %Y %I:%M%p')

@type_checked_constructor(convert=True)
class Foo(NamedTuple):
    msg: str
    timestamp: MyDatetime

json_str = """{
    "msg": "hi",
    "timestamp": "Jun 1 2005  1:33PM"
}"""

foo = Foo(**json.loads(json_str))
print(foo.msg)
print(foo.timestamp.val)

Your proposal is to allow for something along the lines of the following:

def parse_timestamp(repr: str) -> datetime:
    return datetime.strptime(repr, '%b %d %Y %I:%M%p')

@type_checked_constructor(converters={'timestamp': parse_timestamp})
class Foo(NamedTuple):
    msg: str
    timestamp: datetime

converters being of type Dict[str, Callable[[Any], Any]].

Is this correct?

[LukeMathWalker]

Yes, this is exactly what I am proposing ❤️

In particular, the MyDatetime is quite difficult to use in terms of ergonomics: the value is wrapped and you can't use it elsewhere in the codebase as you would do with the primitive type.

[Dobiasd]

OK, will implement. :)

[Dobiasd]

@LukeMathWalker Just pushed a commit that implements it. I've not yet created a new release of the library with it, because I still have to get rid of the feeling to have forgotten about certain corner cases.

Would you mind having a look and let me know what you think? 🙂

[LukeMathWalker]

I noticed an edge case going through these lines: you fire the converter function without first checking if the conversion is necessary.

This is the minimal example I came up with to reproduce the issue:

from typing import NamedTuple
from datetime import datetime
from undictify import type_checked_constructor


def converter(s: str) -> datetime:
    return datetime.strptime(s, '%b %d %Y %I:%M%p')


@type_checked_constructor(
    skip=True, convert=True,
    converters={"timestamp": converter}
)
class MyStruct(NamedTuple):
    timestamp: datetime


if __name__ == "__main__":
    requires_conversion = {"msg": "hi", "timestamp": "Jun 1 2005  1:33PM"}
    _ = MyStruct(**requires_conversion)  # All good

    should_be_good = {"msg": "hi", "timestamp": datetime(2005, 6, 1, 13, 33)}
    _ = MyStruct(**should_be_good)  # Throws a TypeError

[Dobiasd]

Good call, thanks. In my mind, converters had to be used when provided. But I think your definition makes more sense in practice, so I pushed this.

Another implicit decision I made is, that when converters are provided, they can be used even if one did not set convert=True. Do you also think this makes sense?

[LukeMathWalker]

I think it makes sense: conversion might be unavoidable for certain custom types, but I might not want to be "soft" on the rest, so I could use convert=False and set converters for the stuff I need to convert. It provides more fine grained control over the conversion step - I like it 👍

[Dobiasd]

Cool. I just published version 0.6.4. Please let me know about any issues. :)