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

Cannot use self in method params #8

Closed
ghost opened this issue Jul 3, 2014 · 9 comments · Fixed by #9
Closed

Cannot use self in method params #8

ghost opened this issue Jul 3, 2014 · 9 comments · Fixed by #9

Comments

@ghost
Copy link

ghost commented Jul 3, 2014

Hi. Sorry to bother you again. I have the following situation:

class Thing(object):
    @params(self=other, thing=Thing)
    def do_something(self, thing):
        pass

The thing=Thing gives me an "Unresolved reference error". Is there syntax for this already and I'm just missing it? Thanks.

@ghost
Copy link
Author

ghost commented Jul 3, 2014

Oh, also, if I have two classes in the same file which accept instances of each other, I can't list them in the @params because the references cannot be found. I realize this is a Python issue rather than a "typedecorator" issue, but a solution for one may be a solution for all.

@ghost ghost closed this as completed Jul 3, 2014
@ghost
Copy link
Author

ghost commented Jul 3, 2014

Crap, sorry. Didn't mean to close the issue.

@ghost ghost reopened this Jul 3, 2014
@senko
Copy link
Member

senko commented Jul 9, 2014

Hey @RobertPeters, currently we don't have a good support for that. The workaround is, as in the self case, to use object.

In this case it's not a good solution though, since Python doesn't double-check the type (like it does for the self instance for bound methods), so we should do something about them.

Currently I have two ideas on how to solve this, but I don't particularly like any of them, they both feel kind of clunky:

The first idea is to allow specifying the type using strings:

class Thing(object):
     @returns('Thing')
     def just_returns_self(self):
         return self

I don't like this because it's not clear how scoping would work (ie. where to look for the 'Thing' at the moment of type checking), and because it's easy to make a typo that won't show up until the method is executed.

The second idea is to allow functions in type signatures (as described in #5):

class Thing(object):
    @returns(lambda: Thing)
    def just_returns_self(self):
        return self

This has the drawback that it's much uglier, esp. when you've got several of them in @params:

@returns(lambda: Thing)
@params(self=lambda: Thing, other=lambda. Thing)
def __add__(self, other):
    return self.value + other.value

Any feedback on which would work better for your case (or if there's another, better way), please chime in.

@ghost
Copy link
Author

ghost commented Jul 9, 2014

I lean towards the first option because it's cleaner, but I understand what you say about scoping. I'm not too worried about typos because generally you aren't going to find out you have a type error until you run the code anyway.

Would it be possible to make strings valid only for the class they occur in? For example @returns('Thing') is valid inside the class Thing, but not anywhere else?

@senko
Copy link
Member

senko commented Jul 10, 2014

Regarding your question about strings being valid only for enclosing class - yes, (or we could have a special 'self' string that only matches the instance class), but that couldn't be used for checking for instance of other types (for example in the two classes cross-referrencing each other as in your previous comment).

I took the easy (to code and reason about), but not entirely correct, way out: every type has a name attribute. If string is used instead of a type, we just check that the provided string matches the type name (whatever it is and wherever it came from). This means we don't have to worry about scoping

This means that if you have classes bar.Foo and baz.Foo, if you use 'Foo' as the type annotation, it will match both of them.

The implementation is in #9 but I'd like to get some feedback on whether this makes sense before I merge it.

Thoughts?

@ghost
Copy link
Author

ghost commented Jul 10, 2014

I'm no coding genius, but I would be perfectly happy with your suggested solution. I can't think there's too many cases where you would be using types in a new type with the same name. It would be confusing as hell.

Would your solution also mean we would right self='Thing' instead of self=object? If so, I think that's an improvement.

@senko
Copy link
Member

senko commented Jul 11, 2014

Yes, that would work as well.

@senko senko closed this as completed in #9 Jul 11, 2014
@senko
Copy link
Member

senko commented Jul 11, 2014

Fixed in 0.0.4 which I've just published on PyPI.

@ghost
Copy link
Author

ghost commented Jul 11, 2014

Thanks for being so responsive, senko. I'll update and crack on.

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 a pull request may close this issue.

1 participant