Auto-detect user-written methods

[p-leger]

Hey,

I was browsing through the open issues searching for something I could take a stab at implementing.
I came across #324 and thought i would like to try that.

As a a set of 'specifications' i took last comment form @hynek on that issue:

The more I think about it the more I think we should just skip methods that have been defined and be done with it. That allows people to customize their classes even better.

I thought about putting that logic into the attrs method, however I wouldn't have fine enough control over all methods added by add_cmp so decided to move that logic into the ClassBuilder.

I can imagine people are used to attrs overwriting methods by default even if they are defined, thus I would suggest a feature switch (which I haven't implemented yet.).

The tests gave me a bit of trouble, especially when running against Python2.7, since it seems like unbound and bound methods are not eq (please correct me if i misunderstood this).

Furthermore if the Class should be unhashable any user written method is still ignored. However the argument, "A __hash__ method has been supplied, thus the class should be considered hashable" also makes sense to me.

This PR is still very much work in progress, before I spend more time on this, I would greatly appreciated some feedback.

Cheers,

Philipp

Pull Request Check List

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once, it will save you unnecessary review cycles!

• Added tests for changed code.
• New features have been added to our Hypothesis testing strategy.
• Changes or additions to public APIs are reflected in our type stubs (files ending in .pyi).
• Updated documentation for changed code.
• Documentation in .rst files is written using semantic newlines.
• Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives.
• Changes (and possible deprecations) have news fragments in changelog.d.

[hynek]

Heh this is not really an “easy” ticket (although it doesn’t need too much attrs tribal knowledge). The hard part (and problem with your PR) is that you can’t just look whether the method exists but you have to look whether the method exists in the current class vs. somewhere in the inheritance tree.

IOW:

@attr.s
class A:
    x = attr.ib()
    def __init__(self):
        self.x = 42

@attr.s
class B:
    y = attr.ib()

Your code would detect A’s __init__.

Which also kinda poses an interesting question how that should be handled. My gut instinct is to set init=False on A, leave ClassBuilder alone, and make the user call super().

Which takes us to the problem you’ve encountered with cmp. We could say that defining any comparison method disables the creation wholesale. But since we plan to make cmp more granular, that would mean more headaches down the road.

God I don’t have a good answer, but it seems that I won't be able to keep dodging the replacement of bools thru enums much longer. :|

[p-leger]

I suppose you example was meant to be:

@attr.s
class A:
    x = attr.ib()
    def __init__(self):
        self.x = 42

@attr.s
class B(A):
    y = attr.ib()

(If not i misunderstood your point)

I didn't think it would be an easy one, basically I just wanted an topic/problem/excuse to hack on the attrs source code 😁.

If I understood correctly the main issue is methods provided by super classes would also be detected and thus not overwritten by attrs:

I though I had solved that, by doing the following check:

class C:
    def __init__(self):
        pass


class F(C):
    def foo(self):
        pass


print(f'C: {C.__dict__.keys()}')
print(f'F: {F.__dict__.keys()}')

Which gives be the following output:

C: dict_keys(['__module__', '__init__', '__dict__', '__weakref__', '__doc__'])
F: dict_keys(['__module__', 'foo', '__doc__'])

So I am checking for the method in question inside __dict__ for the class which is decorated with attr.s, what am I missing here?

[hynek]

So I am checking for the method in question inside __dict__ for the class which is decorated with attr.s, what am I missing here?

Slotted classes that don’t have a __dict__? 😇

[p-leger]

heh.. yeah indeed. (But in terms of tackling the inheritance problem, it would have worked if it weren't for slotted classes?)

So over the last couple of weeks I have used attr a bit more which got me thinking about this PR...

I started to wonder if the added convenience really does outweigh the complexity/'magic', which would be introduced by implementing it.

So I think I'll just close this PR and see if I can find something which is a bit more straightforward -- any suggestions?

[hynek]

Sorry for not getting back at you any sooner. Yes it might be, that it’s actually not worth the complexity. Sadly it usually isn’t entirely clear until one sees that code/complexity realized. :|