dgilland / sqlservice

The missing SQLAlchemy ORM interface.
https://sqlservice.readthedocs.io
MIT License
179 stars 9 forks source link

__dict_args__ not working as expected #26

Closed adm271828 closed 6 years ago

adm271828 commented 6 years ago

Hello Derrick,

I suggest the last test in _get_dict_adapter to be replaced with:

    try:
        if isinstance(value, cls):
            adapter = _adapter
            break
    except TypeError: pass

This will avoid an exception when __args_dict__ does not provide an adapter for every key, and allow the default adapter to be returned.

class User(Model):
    __tablename__ = 'user'

    __dict_args__ = {
        'adapters': { 'name': lambda v, *_: 'XXX_' + v + '_XXX', }
    }

    id = Column(Integer(), primary_key=True)
    name = Column(String(100))
    email = Column(String(100))

The following works because email is None:

In[34]: User(name='xxx').to_dict()
Out[34]: {'name': 'XXX_xxx_XXX'}

Otherwise, when serializing email, the test ends up with cls being a string ('name') and not a type:

In[35]: User(name='xxx',email='abc').to_dict()
Traceback (most recent call last):

  File "<ipython-input-35-a667e6c41429>", line 1, in <module>
    User(name='xxx',email='abc').to_dict()

  File "C:\Tools\Continuum\Anaconda3\lib\site-packages\sqlservice\model.py", line 215, in to_dict
    class_registry)

  File "C:\Tools\Continuum\Anaconda3\lib\site-packages\sqlservice\model.py", line 356, in _get_dict_adapter
    if isinstance(value, cls):

TypeError: isinstance() arg 2 must be a type or tuple of types

Moreover the documentation says the adapter can be a function that accepts up to three arguments. It seems to me this only work if it accepts exactly three argument (otherwise raise a TypeError and complains it was given 3 arguments).

And wouldn't it be a nice feature to prevent a given field's serialization if the adapter in __dict_args__ is None? For instance:

    __dict_args__ = {
        'adapters': { 'my-secret-field': None }
    }

BTW very nice package. I came to it when trying to reuse a model that was developped inside a Flask application...

Best regards,

Antoine

dgilland commented 6 years ago

Thanks for finding that bug and documentation issue. Both have been fixed in 8dd40fab4fbd1a639bcfec34c346dc234d637d9a and afa73c5be3475b8af4cf17084efe0b27a5f4f6d8.

As for the feature request to allow for fields to be excluded from dict serialization, let me ponder that, and I will get back to you.

adm271828 commented 6 years ago

I see at least 2 use cases:

dgilland commented 6 years ago

I'll go ahead and have this feature added. I might not be able to get to until the weekend so if you wanted to take a stab at a PR, that'd be fine by me. Otherwise, I'll try to get it done soon.

dgilland commented 6 years ago

I've implemented the None adapter support to exclude fields from dict serialization in 97f80f526449839f488663d29ebe04d4220c3dd3 and released in v1.1.3: https://pypi.org/project/sqlservice/1.1.3/

Thanks again for the feature idea and bug report!