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

Change SchemaOpts.json_module to more generic name #364

Closed
justanr opened this Issue Dec 24, 2015 · 5 comments

Comments

Projects
None yet
4 participants
@justanr
Contributor

justanr commented Dec 24, 2015

Seeing as it's not really limited to just json, even if that's most common use case. A more generic name would need to be decided on (raw_handler??) and a deprecation warning could be issued if json_module is set as opposed to the new name.

Use case: I feel silly giving a yaml loader/dumper to json_module.

@sloria

This comment has been minimized.

Member

sloria commented Dec 25, 2015

Yep, this was also suggested in #130 , and I think it's a good idea. We just need to settle on a new name.

So far we have:

  • render_module
  • raw_handler

@sloria sloria added this to the 3.0 milestone Dec 25, 2015

@dirkdevriendt

This comment has been minimized.

dirkdevriendt commented Jan 3, 2017

I would vote for render_module

Though I might actually prefer to be able to pass in a serialize_function and deserialize_function instead of a module that needs to provide functions called "dumps" and "loads".

I don't think pyyaml follows this convention, for instance. And adding a proxy module or including a separate definition for which functions to call may seem overkill?

@maximkulkin

This comment has been minimized.

Contributor

maximkulkin commented Jan 3, 2017

@dirkdevriendt Originally it uses interface that JSON modules provide. For modules that do not support given interface (loads/dumps) you can easily create adapters:

import yaml

# json adapter
class MyJsonSerializer:
    @staticmethod
    def loads(data):
        return yaml.load(data)

    @staticmethod
    def dumps(obj):
        return yaml.dump(data)

# or shorter version
class MyJsonSerializer:
    loads = staticmethod(yaml.load)
    dumps = staticmethod(yaml.dump)


class MySchema(marshmallow.Schema):
    class Meta:
        json_module = MyJsonSerializer
    # rest of schema
@dirkdevriendt

This comment has been minimized.

dirkdevriendt commented Jan 4, 2017

Thanks for the example, I totally understand. I just thought that if the signature is going to be changed anyway, there could be something to say for making the methods themselves configurable. This would enable:

class MySchema(marshmallow.Schema):
    class Meta:
        serialize_function = yaml.dump
        deserialize_function = yaml.load
    # rest of schema

... or any other two methods that may or may not be in the same module.

@sloria

This comment has been minimized.

Member

sloria commented Mar 18, 2017

Since the "render module" decides the behavior of "dumps" and "loads", I think it is simpler to have just one option that assumes a "dumps" and "loads" method.

As for the name, I am leaning towards render_module.

@sloria sloria modified the milestones: 3.0.0b2, 3.0 Mar 19, 2017

@sloria sloria closed this in 3331f83 Mar 26, 2017

rowillia added a commit to rowillia/marshmallow that referenced this issue Aug 3, 2017

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