Skip to content

Add section on methods equality and identity #241

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 8, 2020
Merged

Add section on methods equality and identity #241

merged 2 commits into from
Nov 8, 2020

Conversation

Jongy
Copy link
Contributor

@Jongy Jongy commented Nov 3, 2020

@satwikkansal ,

That's interesting. We can possibly add following based on the SO answer you linked,

c1.__str__ == c2.__str__  # False
c1.__dict__.__str__ == c2.__dict__.__str__ # True ??

I decided to skip this particular snippet, because:

  1. The explanation is already quite long 😅 And I think explaining these bits about descriptors and differences between classmethod, staticmethod etc is more useful for Python developers, compared to understanding the (funny) c1.__dict__.__str__ o.O
  2. It irrelevant in modern Python versions (does not reproduce on 3.8)

Closes: #233

satwikkansal
satwikkansal reviewed Nov 4, 2020
View reviewed changes
Copy link
Owner

@satwikkansal satwikkansal left a comment

Choose a reason for hiding this comment

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

Thanks, @Jongy for the PR. The example is well-thought out and looks pretty good. I just added comments for minor suggestions, feel free to check them out whenver you get time.

And yes, you're right about the __str__ method snippet, makes sense to skip it (I didn't knew it doesn't work with mordern python versions).

README.md
>>> print(o1.staticm is o1.staticm is o2.staticm is SomeClass.staticm)
True
```

Copy link
Owner

Choose a reason for hiding this comment

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

To set up more clearly, can we add a sentence along the lines of

Accessing classm or method twice creates "equal but not same" objects for instance of SomeClass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, added.

README.md
`SomeClass.method is SomeClass.method` is truthy.
* `classmethod` transforms functions into class methods. Class methods are descriptors that, when accessed, create
a method object which binds the *class* (type) of the object, instead of the object itself.
* Unlike functions, `classmethod`s will create a method also when accessed as class attributes (in which case they
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't it be "Like" instead of "Unlike"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, its the difference between the function descriptor (func_descr_get) which, when invoked on the class (as opposed to an instace), returns the function itself and not a method; while the classmethod descriptor (cm_descr_get) will create a method also when invoked directly on a type object (e.g a class).

Perhaps my phrasing isn't clear?

Copy link
Owner

Choose a reason for hiding this comment

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

It's fine, I misread the also part, my bad.

README.md Outdated
a method object which binds the *class* (type) of the object, instead of the object itself.
* Unlike functions, `classmethod`s will create a method also when accessed as class attributes (in which case they
bind the class, not to the type of it). So `SomeClass.classm is SomeClass.classm` is falsy.
* A method object compares equal when both the functions are equal, and the bound objects are one. So
Copy link
Owner

Choose a reason for hiding this comment

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

For clarity, I think we can say "the bound objects are same" instead of "the bound objects are one".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, reads better.

@Jongy
Copy link
Contributor Author

Jongy commented Nov 4, 2020

Thanks, @Jongy for the PR. The example is well-thought out and looks pretty good. I just added comments for minor suggestions, feel free to check them out whenver you get time.

Thanks, I did take my time organizing it, since it turned out pretty heavy.

By the way - I just noticed there already is a section about this behavior, Non-reflexive class method. My bad for forgetting it...

To remove redundancy, perhaps we want to merge them / remove one of them, while keeping all notes?

@satwikkansal
Copy link
Owner

To remove redundancy, perhaps we want to merge them / remove one of them, while keeping all notes?

Ah, right, I totally forgot about that. But now that I see it, your example is a superset of the existing one (except for the id(SomeClass.class_method) == id(SomeClass.class_method) part). I guess we can update the existing one with the snippets you've added.

I know merging might make it slightly longer, but it's fine as long as it's interesting and non-redundant.

@Jongy
Copy link
Contributor Author

Jongy commented Nov 7, 2020

To remove redundancy, perhaps we want to merge them / remove one of them, while keeping all notes?

Ah, right, I totally forgot about that. But now that I see it, your example is a superset of the existing one (except for the id(SomeClass.class_method) == id(SomeClass.class_method) part). I guess we can update the existing one with the snippets you've added.

I know merging might make it slightly longer, but it's fine as long as it's interesting and non-redundant.

I will look into merging it. About repeating the id(..) == id(..) - this bit is already explained in https://github.com/satwikkansal/wtfpython#-deep-down-were-all-the-same, so I don't see any reason to repeat it.

@Jongy
Copy link
Contributor Author

Jongy commented Nov 7, 2020

Only part that's missing is this:

>>> SomeClass.instance_method
<function __main__.SomeClass.instance_method(self)>
>>> SomeClass.class_method
<bound method SomeClass.class_method of <class '__main__.SomeClass'>

I'll merge it into my example, alright?

@Jongy
Copy link
Contributor Author

Jongy commented Nov 7, 2020

@satwikkansal Merged them, decided to keep the "new" name since it's more relevant IMO, tell me what you think of this change.

@satwikkansal
Copy link
Owner

Looks good now, thanks, merging it!

@satwikkansal satwikkansal merged commit c954b71 into satwikkansal:master Nov 8, 2020
@Jongy Jongy deleted the methods branch November 8, 2020 08:06
@Jongy
Copy link
Contributor Author

Jongy commented Nov 8, 2020

Thanks @satwikkansal :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@satwikkansal satwikkansal satwikkansal approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Methods, classmethods and staticmethods
2 participants
@Jongy @satwikkansal