Add decorator-based pre/post-dump/load support #191
Conversation
|
Two open questions:
|
I agree. |
|
@taion Thanks for your work on this. I will look at this more in-depth over the weekend. |
| mro = klass.mro() | ||
|
|
||
| klass.processors = defaultdict(list) | ||
| for attr_name in dir(klass): |
There was a problem hiding this comment.
dir apparently returns an alphabetical list of attributes, meaning that decorated methods might not be applied in order expected by the developer. You might use inspect or store a counter on the decorators to get the right order.
There was a problem hiding this comment.
Do you think we need to guarantee order beyond "raw processors last when dumping, raw processors first when loading"? I feel that this would not be an especially obvious feature, and that ordering dependencies would be better expressed by putting them in the same processor, rather than depending on some guaranteed order.
There was a problem hiding this comment.
Agreed that users should handle order by putting ordered operations in the same callback. But they might not, and if they don't, I could imagine this being a debugging headache. Anyway, if we don't want callbacks to run in any particular order, the docs should be clear about that, so that users don't make assumptions and get confused.
There was a problem hiding this comment.
I'd be inclined to go with telling users not to depend on any ordering guarantees in the docs. It just seems like not the best idea. @sloria what do you think?
There was a problem hiding this comment.
Yes, I think the docs should clearly state that ordering is not guaranteed.
| POST_LOAD = 'post_load' | ||
|
|
||
|
|
||
| def pre_dump(*args, **kwargs): |
There was a problem hiding this comment.
For documentation purposes, it might be clearer to explicitly include raw in these signatures. The tag_processor includes the raw value in the tuple tags anyway.
|
Looking good so far @taion . Let me know when this is ready for further review. |
|
New commit includes (2) from my 1st comment about treating |
| # the class, we let standard inheritance do all the hard work. | ||
| mro = klass.mro() | ||
|
|
||
| klass.__processors__ = defaultdict(list) |
There was a problem hiding this comment.
Can this "processor tag" logic be put in a separate helper function or method?
|
Just a minor nitpicky refactor, and this should be good to merge! |
|
Done. Is this for 2.x, or will there be a 1.3 line that includes new features without removing old ones? |
|
Thanks @taion . This will go into 2.0-a. |
Add decorator-based pre/post-dump/load support
For #153