Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow easier skipping of loading and dumping of an attribute #90

Open
altendky opened this issue Feb 24, 2020 · 8 comments
Open

Allow easier skipping of loading and dumping of an attribute #90

altendky opened this issue Feb 24, 2020 · 8 comments

Comments

@altendky
Copy link
Member

@sveinse brought up in #python yesterday their use case of a full desert class where the serialization is only used to pass a subset of the attributes to a remote GUI. I think this is presently supported by manually creating a marshmallow field.

do_not_serialize: str = desert.ib(marshmallow.Fields.String(dump_only=False, load_only=False))

But that's pretty unwieldy and WET.

I proposed the following interface that seems pretty straightforward to implement and use.

do_not_serialize: str = desert.ib(dump=False, load=False)

For completeness, I also proposed an alternative that is unlikely to end up a good choice but it relates to my frustration that 'annotations' have been co-opted to exclusively hold 'type hints' instead of retaining a more general role of annotating things. This could include serialization data, documentation strings, probably lots more. But, that's not what anyone does...

do_not_serialize: desert.Ignore[str]

or... something. It's unlikely to be a thing so I didn't put much thought into how it would be made to not inhibit regular type hints checking or any alternatives etc.

As to implementation of the desert.ib() option, I got so far as considering that the marshmallow_field parameter ought to be optional and if not specified the regular inspection processes should be used to create the field.

def ib(marshmallow_field: marshmallow.fields.Field, **kw) -> attr._make._CountingAttr:
"""Specify a marshmallow field in the metadata for an ``attr.dataclass``.
.. code-block:: python
@attr.dataclass
class A:
x: int = desert.ib(marshmallow.fields.Int())
"""
meta = metadata(marshmallow_field)
meta.update(kw.pop("metadata", {}))
return attr.ib(**kw, metadata=meta)

I'm thinking that the following two attributes should be treated the same way. After that, extra info can be passed to desert.ib() to adjust the otherwise automatic marshmallow field creation.

x: str
y: str = desert.ib()

Or, maybe my limited exposure to desert has left me overlooking existing related features...

@python-desert
Copy link
Collaborator

python-desert commented Feb 24, 2020

But that's pretty unwieldy and WET.

I proposed the following interface that seems pretty straightforward to implement and use.

As you say,

do_not_serialize: str = desert.ib(marshmallow.fields.String(dump_only=False, load_only=False))

is verbose, and repetitive since marshmallow.fields.String() could be inferred from str. I think Marshmallow itself would be better off with dump= and load= instead of dump_only= and load_only=, but they've had that since Marshmallow 2.0.

There are four different kinds of data we're specifying.

  1. The type hint for the attrs class, like str.
  2. The Marshmallow field type, like marshmallow.fields.String().
  3. The attr.ib() arguments, like repr=False.
  4. The marshmallow.fields.String() arguments, like dump_only=True or data_key="fooBar".

In a solution like

do_not_serialize: str = desert.ib(dump=False, load=False)

where do repr=False and data_key="fooBar" go? Do we want to combine the marshmallow.fields.String() parameters and attr.ib() parameters into a single basket that takes repr=True and data_key="fooBar"? If so, do we need to add attr_metadata={} and marshmallow_metadata={} parameters, or are we merging those together in a single metadata={}?

Just to get the ideas flowing....we could have:

foo_bar: str = desert.thing(marsh(data_key="fooBar"), ib(repr=False))

or

foo_bar: str = desert.thing(marsh=dict(data_key="fooBar"), ib=dict(repr=False))

One difference between these is that the latter permits at most one of each marsh/ib, while the former could, at least syntactically, take more. The keyword arguments suggest that Desert knows about marsh and ib specifically.

What happens if we want to specify other representations? For example, an SQLAlchemy column type, a PyQt/Toga widget, a Deform/WTForms HTML form, and a Click CLI option?

@python-desert
Copy link
Collaborator

For completeness, I also proposed an alternative that is unlikely to end up a good choice but it relates to my frustration that 'annotations' have been co-opted to exclusively hold 'type hints' instead of retaining a more general role of annotating things. This could include serialization data, documentation strings, probably lots more. But, that's not what anyone does...

do_not_serialize: desert.Ignore[str]

Perhaps you have in mind something like the accepted PEP 593.

@altendky
Copy link
Member Author

Is desert trying to create an abstraction layer? Or just a layer that merges attrs and marshmallow together?

Where sure, even an abstraction layer can be well served by letting you poke through it when you want, like specifying the exact marshmallow.Field to use.

@python-desert
Copy link
Collaborator

I've edited my comments above with some more material.

@python-desert
Copy link
Collaborator

Is desert trying to create an abstraction layer? Or just a layer that merges attrs and marshmallow together?

What would be the implications of each of these options?

@python-desert
Copy link
Collaborator

python-desert commented Feb 24, 2020

We can reduce the repetitiveness, if not the verbosity, by replacing the invocation of a specific Marshmallow field type with some indirection:

foo_bar: str = desert.ib(desert.inferred_marshmallow_field(data_key="fooBar"), repr=False)

where the inferred_marshmallow_field() is replaced with marshmallow.fields.String() because of the str annotation.

@sveinse
Copy link
Collaborator

sveinse commented Feb 24, 2020

Let me permit to show how I use it in my application, because I hope it might be relevant for the discussion how to use attr, desert and marshmallow and ways to pass arguments to them all.

# The metadata field of attr.ib() is used to convey application
# specific information. In this case UI access control, which is done
# though dedicated ib function variants.
bit_error: str = rw_param(default='Zero')

# When using types not known to marshmallow, fields must added
matrix: Matrix = rw_param(field=MatrixField)

# ...by adding the field as a keyword to the custom ib function
# Note the selection between attr.ib() and desert.ib(). This is because
# desert.ib() requres a field, but for stock types it is not needed.
def ib(**kw):
    field = kw.pop("field", None)
    if field is not None:
        return desert.ib(field(), **kw)
    return attr.ib(**kw)

# Example overload of the ib() function
def rw_param(**kw):
    ''' UI accessible read-write parameter '''
    metadata = {'ro': False, 'ui': True}
    metadata.update(kw.pop("metadata", {}))
    return ib(metadata=metadata, **kw)

In my use-case I can use the field.metadata['iu'] attribute to select what shall be serialized. As proposed by @altendky I can easily add other keywords to my custom ib() functions that will control the serializer.

@altendky
Copy link
Member Author

I feel like https://github.com/altendky/exttr (the two-days-of-dev library, or the idea) may play in as an additional higher level layer, if anyone wanted to consider it. Maybe anyways...

An abstraction layer wouldn't make you decide that the option you are passing is for marshmallow. The option would simply be a feature provided by desert, documented by desert, and implemented however is needed. If instead of abstracting the 'merge' route is taken then yeah, some form of pass through to attrs or marshmallow as specified by the caller would be relevant. The more you want to avoid making the user learn the underlying packages or the more you want to support multiple back-ends then the more you need to create your own interface that doesn't leak the underlying details out.

But sure, even when abstracting... sometimes people want to poke through so it's nice to allow that. I can imagine a few ways. Just accept a marshmallow field, end of story. Accept some parameters to fill in extra details. Accept parameters that override anything desert would otherwise pass to marshmallow.

My gut is tending towards abstracting because it should get you the most integrated and least verbose (and least WET) solution. But sure, it is a hazardous road to get right.

Unless we are going the exttr(-style) route, I would say that the metadata should go under desert's key. Once there, it's not a huge deal what form it is in since we can refactor/correct it at any time (being internal and all). Also, the attrs stuff doesn't go into metadata, does it?

Are you suggesting several 'backends' at once with the PyQt/sqlalchemy/... thought? If they are going to work similarly they can sit behind one set of metadata (similar like 'all backends will not serialize attribute x'). If they are going to behave differently (interface X doesn't serialize y but interface z does but skips loading of a...) then sure, you need per-backend configuration. I suppose you would get towards something more akin to your desert.ib(this(), that()) option. Though perhaps formed more like...

x: str = desert.ib(
    repr=False,
    configurations={
        a_key: desert.backend_ib(
            dump=False,
        ),
        something_else: desert.backend_ib(
            load=False,
        ),
    },
)

When you go to create a schema you would specify 1) what backend to create it for (marshmallow, sqlalchemy, etc) and 2) which configuration key (a_key etc) to use to get the configuration. We could allow a shortcut if you use the package objects as the keys (maybe a terrible idea?) and then you could specify the backend and the configuration choice in one go. Like desert.schema(MyClass, backend=marshmallow) would implicitly use marshmallow as the configuration key as well.

I will say that my experience (with graham and PyQt model 'backends') does make me think there's a point where you don't want this integrated into one class definition. A little repetition in trade for separation can make things more legible. Part of that is likely my stuff being a mess, so I don't mean that two backends is too much, but there is a point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants