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

Dynamic Defaults Using New Decorator API #76

Closed
wants to merge 1 commit into from

Conversation

rmorshea
Copy link
Contributor

@rmorshea rmorshea commented Sep 4, 2015

brief example

class A(HasTraits):
    i = Int()

    @default('x')
    def _x_default(self):
        return 1

A().i

result : 1

@rmorshea
Copy link
Contributor Author

rmorshea commented Sep 4, 2015

builds on #73 to expand the decorator api and replace legacy magic methods

@rmorshea rmorshea force-pushed the observe_validate_default branch 9 times, most recently from 89bee56 to a9f90a4 Compare September 8, 2015 22:32
@rmorshea
Copy link
Contributor Author

rmorshea commented Sep 8, 2015

@SylvainCorlay, @jasongrout this pr is ready for review.

@SylvainCorlay
Copy link
Member

I guess that you will want the same mechanism as for the validate handler to avoid the registration of two default handlers and handle the case where the old method is registered with the decorator.

I think we should probably wait for the other PR to be merged before getting more into this one.

meth = types.MethodType(self.func, inst)
inst._register_validator(meth, self._name)

class DefaultHandler(EventHandler):
Copy link
Member

Choose a reason for hiding this comment

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

The setting and getting logic in this class needs to be audited @SylvainCorlay

@SylvainCorlay SylvainCorlay mentioned this pull request Sep 13, 2015
@SylvainCorlay
Copy link
Member

This needs a rebase.

@rmorshea rmorshea force-pushed the observe_validate_default branch 4 times, most recently from 69cb14c to 6aecb07 Compare September 15, 2015 21:14
@@ -372,6 +372,8 @@ def __init__(self, default_value=Undefined, allow_none=None, read_only=None, hel
if help is not None:
self.metadata['help'] = help

self.default_handlers = []
Copy link
Member

Choose a reason for hiding this comment

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

Since there are already _trait_notifiers, _trait_validators and _trait_values, this could be made private and called _trait_default_generators or something like this. Never mind.
Why don't you register default handlers at the HasTraits instance level?

value = getattr(cls, key, None)
if value and isinstance(value, DefaultHandler):
if self.name in value.names:
return getattr(obj, event_handler.name)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SylvainCorlay removed the need for storage of EventHandlers on TraitType instances and allowed multiple trait names in the default decorator as in observe and validate.

value = getattr(cls, key, None)
if value and isinstance(value, DefaultHandler):
if self.name in value.names:
return getattr(obj, value.name)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SylvainCorlay new dynamic default decorator logic.

@minrk minrk modified the milestone: 4.1 Oct 5, 2015
@rmorshea rmorshea force-pushed the observe_validate_default branch 4 times, most recently from bbcbe99 to d443215 Compare October 6, 2015 07:58
@rmorshea rmorshea closed this Oct 7, 2015
@rmorshea rmorshea reopened this Oct 7, 2015
@SylvainCorlay SylvainCorlay mentioned this pull request Oct 9, 2015
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

4 participants