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

Subclass support for handlers + decorator syntax for register() #104

Merged
merged 2 commits into from Dec 29, 2014

Conversation

aldanor
Copy link
Contributor

@aldanor aldanor commented Dec 29, 2014

This PR adds two features, both being improvements to the jsonpickle.handlers.register function:

  1. An optional keyword argument base, disabled by default. If set to True, the handler will be invoked for all subclasses of the given class. If a class has a personal handler that was set explicitly, it will always take precedence (see tests).
  2. Optional decorator syntax:
@jsonpickle.handlers.register(Foo, base=True)
class FooHandler(jsonpickle.handlers.BaseHandler):
    pass

// there are also some minor pep8 fixes thoughout.

def get(self, class_name):
return self._handlers.get(class_name)
def get(self, cls_or_name, default=None):
handler = self._handlers.get(cls_or_name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. A docstring would be warranted now this is a little more complex.

@marcintustin
Copy link
Contributor

This is ace!

Unfortunately, I don't have commit access, so @davvid will have to handle it.

One thing you can do to make it smoother is to ensure that travis-ci is enabled for your fork, and that all tests pass.

@aldanor
Copy link
Contributor Author

aldanor commented Dec 29, 2014

No idea how to enable travis building the fork.

Btw: why don't you guys use something like tox to simplify local testing vs different interpreter versions?

@marcintustin
Copy link
Contributor

@aldanor https://travis-ci.org/ It's pretty self-explanatory.

I'm not sure I see the advantages of tox, given the features of travis as a continuous integration tool.

@aldanor
Copy link
Contributor Author

aldanor commented Dec 29, 2014

Ok I've pushed some minor changes and enabled the Travis build; looks all green.

Re: tox, you can run tox locally with a single keypress and ensure the tests pass on all possible interpreter versions (and dependency package versions, if need be); many major Python packages make use of that (e.g. pandas, numpy, flask just to name a few off top of my head).

Basically, this simplifies the process of testing things thoroughly before you push them, and lets you test the package exactly as it would be tested remotely on travis.

davvid added a commit that referenced this pull request Dec 29, 2014
Subclass support for handlers + decorator syntax for register()

Signed-off-by: David Aguilar <davvid@gmail.com>
@davvid davvid merged commit cd7eee3 into jsonpickle:master Dec 29, 2014
@aldanor
Copy link
Contributor Author

aldanor commented Dec 29, 2014

Thanks @davvid 👍

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

Successfully merging this pull request may close these issues.

None yet

3 participants