Add support for field converters #491

Activity

1. jcrist commented on Jul 26, 2023

   

   jcrist

   on Jul 26, 2023

   Last edited by jcrist

   Member

   More actions

   Thanks for the kind words! I believe the main use case here (custom validation/transformation functions) is a duplicate of #483. If so, I'd prefer to close this one and track that work there.

   Sometimes datetimes are encoded as unix-timestamps in miliseconds, so msgspec's automatic parsing does not work.

   With strict=False we now support parsing unix timestamps (in seconds) to datetimes. If this is your only use case, I could be convinced to adopt pydantic's pragmatic approach of assuming big integers in datetime fields are in milliseconds not seconds. It feels a bit sloppy, but software is meant to be useful and that would be an easy feature to add.

   ---

   The best way to handle your problem today is to define a subclass of the types you want to customize, use those in the annotations, and use the existing dec_hook/enc_hook mechanisms to override how serialization works. This has a bunch of caveats (namely custom types like this don't work with unions), but if it works for you this would be more efficient and simpler than your current approach. Something like:

   import datetime
   
   import msgspec
   
   
   class unixms(datetime.datetime):
       pass
   
   
   def enc_hook(x):
       if isinstance(x, unixms):
           return x.timestamp() * 1000
       raise NotImplementedError
   
   
   def dec_hook(typ, x):
       if typ is unixms:
           return unixms.fromtimestamp(x / 1000, datetime.timezone.utc)
       raise NotImplementedError
   
   
   class LoginEvent(msgspec.Struct):
       name: str
       timestamp: unixms
   
   
   msg = b'{"name": "alice", "timestamp": 1690346418357.0}'
   
   
   event = msgspec.json.decode(msg, type=LoginEvent, dec_hook=dec_hook)
   print(event)
   #> LoginEvent(
   #>   name='alice',
   #>   timestamp=unixms(2023, 7, 26, 4, 40, 18, 357000, tzinfo=datetime.timezone.utc)
   #> )
   
   msg2 = msgspec.json.encode(event, enc_hook=enc_hook)
   print(msg2)
   #> b'{"name":"alice","timestamp":1690346418357.0}'
2. jcrist commented on Jul 26, 2023

   

   jcrist

   on Jul 26, 2023

   Member

   More actions

   EDIT: Actually, modifying repr for fields would also be a nice thing you can do in attrs and I frequently use. If you were up to adding that aswell, I'd gladly open a new issue

   I'm curious how you envision that working. If you wouldn't mind opening an issue, I'd be happy to discuss there. No promises it'll be implemented, but there's no harm in asking.

   ---

   Also, not sure if you're aware, but msgspec supports encoding/decoding attrs types natively: https://jcristharif.com/msgspec/supported-types.html#attrs.

   For new code there's not a strong reason to use attrs with msgspec (Struct types are much more performant and have extra features), but for existing code bases the hope is this allows for a smoother transition to using msgspec. Since you're an attrs user, I'd value some feedback on our attrs integration - there may be some rough edges/missing features here that make using attrs with msgspec less useful than it could be.
3. aedify-swi commented on Jul 26, 2023

   

   aedify-swi

   on Jul 26, 2023

   Last edited by aedify-swi

   Author

   More actions

   Thanks for the kind words! I believe the main use case here (custom validation/transformation functions) is a duplicate of #483. If so, I'd prefer to close this one and track that work there.

   Did not find that one, but yes that's basically what I am asking for. You can close this one! Well now that I read again, would you want to have both validation and conversion in one function? It surely works, but to me it would feel better so separate them. I do like the way attrs.field handles this.

   The best way to handle your problem today is to define a subclass of the types you want to customize, use those in the annotations, and use the existing dec_hook/enc_hook mechanisms to override how serialization works. This has a bunch of caveats (namely custom types like this don't work with unions), but if it works for you this would be more efficient and simpler than your current approach. Something like:

   This works but is also a wee less convenient. Also I personally don't like that I have to subclass a standard type.

   Also, not sure if you're aware, but msgspec supports encoding/decoding attrs types natively: https://jcristharif.com/msgspec/supported-types.html#attrs.

   I know, but the attrs field converter function does not get called, when I use convert.

   Thank for your replies! I'll open a new ticket re repr customisation.
4. aedify-swi commented on Jul 26, 2023

   

   aedify-swi

   on Jul 26, 2023

   Author

   More actions

   For new code there's not a strong reason to use attrs with msgspec (Struct types are much more performant and have extra features), but for existing code bases the hope is this allows for a smoother transition to using msgspec. Since you're an attrs user, I'd value some feedback on our attrs integration - there may be some rough edges/missing features here that make using attrs with msgspec less useful than it could be.

   I am aware and I am using it. I'll check this, but I think using msgspec.convert does not trigger the attrs field converters (maybe using the attributes flag).

   Actually, the attrs integration made me start using msgspec, because I find cattrs unnecessarily complicated and using msgspec to convert sqlalchemy.Row objects into our attrs domain models works very well.

   From my perspective the only blockers to fully switch to msgspec.Structs are the repr, converter and validator functionalities of attrs.field.
5. aedify-swi commented on Jul 26, 2023

   

   aedify-swi

   on Jul 26, 2023

   Last edited by aedify-swi

   Author

   More actions

   I checked again and this is the current behaviour, which might be considered a bug (if you want to fully support attrs classes):

   from dataclasses import dataclass
   
   import attrs
   import msgspec
   
   
   @attrs.define
   class AttrsClass:
       string: str = attrs.field(converter=lambda x: str(x))
   
   
   @dataclass
   class VanillaClass:
       string: int
   
   
   print(msgspec.convert(VanillaClass(string=1), AttrsClass, from_attributes=True))
   # Exception has occurred: ValidationError
   # Expected `str`, got `int` - at `$.string`
   
   print(msgspec.convert({"string": 1}, type=AttrsClass))
   # Exception has occurred: ValidationError
   # Expected `str`, got `int` - at `$.string`
   
   print(AttrsClass(string=1))
   # AttrsClass(string='1')
6. jcrist commented on Jul 26, 2023

   

   jcrist

   on Jul 26, 2023

   Member

   More actions

   Thanks for the replies. Can you respond to this bit? If this is your only current blocker it'd be a quick thing to fix:

   If this is your only use case, I could be convinced to adopt pydantic's pragmatic approach of assuming big integers in datetime fields are in milliseconds not seconds. It feels a bit sloppy, but software is meant to be useful and that would be an easy feature to add.

   ---

   Well now that I read again, would you want to have both validation and conversion in one function? It surely works, but to me it would feel better so separate them. I do like the way attrs.field handles this.

   I suspect we'd want a few hooks (probably one for before the builtin conversion/validation bits run and one for after). If you have feedback on the design please comment on that issue.

   I'll check this, but I think using msgspec.convert does not trigger the attrs field converters (maybe using the attributes flag).

   To clarify a bit, msgspec.convert and the decode functions all provide the same conversion/validation paths. The simplest demo of the behavior you're talking about would be:

   In [1]: import msgspec, attrs
   
   In [2]: @attrs.define
      ...: class Example:
      ...:     x: int
      ...:     y: int = attrs.field(converter=int)
      ...: 
   
   In [3]: msg = b'{"x": 1, "y": 2}'  # a valid message
   
   In [4]: msgspec.json.decode(msg, type=Example)
   Out[4]: Example(x=1, y=2)
   
   In [5]: msg2 = b'{"x": 1, "y": "2"}'  # an invalid message
   
   In [6]: msgspec.json.decode(msg2, type=Example)
   ---------------------------------------------------------------------------
   ValidationError                           Traceback (most recent call last)
   Cell In[6], line 1
   ----> 1 msgspec.json.decode(msg2, type=Example)
   
   ValidationError: Expected `int`, got `str` - at `$.y`

   We currently don't do anything with attrs converters. My reasoning was that IMO converters are for handling flexibility on the python side (so users can call Example(1, "2") and have things "just work"). But when decoding they felt like the wrong place to handle this logic. You might want your python code to be able to construct an attrs object from a wider set of types than you'd want to support in a REST api - coupling the types supported in __init__ to the types supported in decode felt wrong. That said, I'm not an attrs user so I'm not 100% sure what's expected behavior here.
7. aedify-swi commented on Jul 26, 2023

   

   aedify-swi

   on Jul 26, 2023

   Author

   More actions

   Thanks for the replies. Can you respond to this bit? If this is your only current blocker it'd be a quick thing to fix:

   If this is your only use case, I could be convinced to adopt pydantic's pragmatic approach of assuming big integers in datetime fields are in milliseconds not seconds. It feels a bit sloppy, but software is meant to be useful and that would be an easy feature to add.

   I wouldn't do that. I'd rather wait for a more flexible way to convert values. That was just one example. I sometimes have to strip prefixes from strings and other shenanigans, so that would be way more helpful in general.

   We currently don't do anything with attrs converters. My reasoning was that IMO converters are for handling flexibility on the python side (so users can call Example(1, "2") and have things "just work"). But when decoding they felt like the wrong place to handle this logic. You might want your python code to be able to construct an attrs object from a wider set of types than you'd want to support in a REST api - coupling the types supported in init to the types supported in decode felt wrong. That said, I'm not an attrs user so I'm not 100% sure what's expected behavior here.

   I can't speak for the wider attrs user community, but I would expect those converters to be called when using msgspec.convert. I for one would never use them for inputs from the python side, because with type hints and all I am pretty clear on what a class expects me to pass. Instead, I am primarily using it for converting non-standard inputs (str -> UUID7) from users (REST API users that is) or for converting remote API data, that needs to be converted into my schema or python types. There is one API we're calling to get tracking data, for example, that sends IDs as ints in their REST API, even though I know it is a String ID in their system. So my converter takes that int, converts it to str and left-pads it with zeros.

   To be honest, about 90% of my converters is now obsolete because msgspec automatically does the conversion (eg. int -> datetime, str-> int in lax mode), but for the few remaining cases these small converters are very helpful.

   👍React with 👍1Reacted by wikiped
8. jcrist commented on Jul 26, 2023

   

   jcrist

   on Jul 26, 2023

   Member

   More actions

   Thanks for the context! I've split the attrs converters bit out into #494. I'm going to close this specific issue in favor of #483 instead.
9. jcrist

   closed this as completedon Jul 26, 2023