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

Why is verify*'s argument-name-code commented out? #63

Open
Julian opened this issue Nov 16, 2016 · 6 comments
Open

Why is verify*'s argument-name-code commented out? #63

Julian opened this issue Nov 16, 2016 · 6 comments
Labels

Comments

@Julian
Copy link

Julian commented Nov 16, 2016

Hi!

I noticed that zope.interface.verify* are not verifying argument names.

It appears that https://github.com/zopefoundation/zope.interface/blob/master/src/zope/interface/verify.py#L108-L111 is commented out, but I can't find why in the history.

Is there a reason why that's been commented out (seemingly since 2007)?

(Sample code that unexpectedly passes:

from zope.interface import Interface, implementer, verify
class IFoo(Interface):
    def method(a, b):
        pass

@implementer(IFoo)
class Foo(object):
    def method(self, c, d):
        pass

verify.verifyObject(IFoo, Foo())

)

@Julian
Copy link
Author

Julian commented Nov 16, 2016

(Sorry -- to elaborate, I know that signature comparison is somewhere between annoying, hard, and impossible :) -- so perhaps the real question is "would anyone like a patch to make such a thing enabled-by-default-but-toggleable-off", if the reasoning here was something to do with that fragility)

@icemac
Copy link
Member

icemac commented Nov 18, 2016

@Julian It seems this code was already commented out when it was added to Zope3 more than 13 years ago, see

There must have been a CVS repository of Interface before where the code was taken from but http://cvs.zope.org is no more.

@icemac icemac closed this as completed Nov 18, 2016
@icemac icemac reopened this Nov 18, 2016
@tseaver
Copy link
Member

tseaver commented Dec 6, 2016

@Julian Memory eludes me, as well. Perhaps @jimfulton has a notion?

@jimfulton
Copy link
Member

I don't remember the occasion of commenting this out, but I don't think it's a good idea to verify positional argument names, because they are positional. :) I'm guessing that this was commented out because it caused breakage.

Python doesn't make a clean distinction between positional and named arguments (although Python 3 allows definition of arguments that must be provided by name). Often positional argument are obscured discourage their use by name, but this is informal.

@Julian
Copy link
Author

Julian commented Dec 7, 2016

@jimfulton I agree that verifying positional arguments is not very useful. In any library I create (and in libraries I see from people who have enough detail generally to even care about these things), argument names I think do tend to be part of their stable public API -- i.e., one promises not to rename def func(foo, bar) to def func(foo2, bar).

But I personally don't ever guarantee not to reorder arguments (mostly because I discourage passing arguments positionally pretty much ever personally).

So for my own things I'd always want to verify the names of arguments in an interface, and never their position.

I can definitely understand wanting to not verify either thing though, especially since I certainly don't think this particular point is paid a ton of attention by library authors, but how do you feel about making it toggleable?

@adiroiban
Copy link

Maybe have something like verify.verifyObject(IFoo, Foo(), positional=True)
But it looks like this is now checked by mypy... so mabye this is now of a lower priority in zope.

rodrigc added a commit to twisted/twisted that referenced this issue Jul 9, 2020
[mypy] [9897] Rename parameters to match interface definitions

This is related to these issues in zope.interface:

* zopefoundation/zope.interface#63
* zopefoundation/zope.interface#140

Author: rodrigc
Reviewer: adiroiban
Fixes: ticket:9897
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants