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

sopel: use python3 style class and abc.ABC #2138

Merged
merged 3 commits into from
Sep 23, 2021

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Jul 2, 2021

Description

In Python 2, there was 2 styles:

  • class ClassName:
  • class ClassName(object)

And they behaved differently, and using super() is different too. However, in Python3, both are equivalent to doing the second style, and we can use super() directly. I remember having to work with Py2.4 compatible code in my career, and this "new style class" stuff was... a pure nightmare. I'm just glad we can get rid of this.

Also, there is now abc.ABC that allows to describe an abstract class as being abstract. This is interesting because it doesn't require to add raise NotImplementedError anymore: a few decorators and it's all good!

One last thing: I added few type hint here and there, and I had fun with TypeVar. It's looking good!

Note: I think I want a "housekeeping" or "cleaning code" label or something. 😁

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

@Exirel Exirel added the Tweak label Jul 2, 2021
@Exirel Exirel added this to the 8.0.0 milestone Jul 2, 2021
@Exirel Exirel requested a review from a team July 2, 2021 22:00
@dgw dgw added the Housekeeping Code cleanup, removal of deprecated stuff, etc. label Jul 2, 2021
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

I could resolve the merge conflict manually (it's trivial), but there are a few other things.

sopel/tools/_events.py Outdated Show resolved Hide resolved
sopel/plugins/handlers.py Show resolved Hide resolved
sopel/plugins/handlers.py Show resolved Hide resolved
sopel/plugins/rules.py Outdated Show resolved Hide resolved
sopel/plugins/rules.py Outdated Show resolved Hide resolved
@Exirel Exirel force-pushed the class-py3-style branch 2 times, most recently from 53caa7d to dc836bc Compare July 5, 2021 07:21
In Python 2, there was 2 styles:

* `class ClassName:`
* `class ClassName(object)`

And they behaved differently, and using `super()` is different too.
However, in Python3, both are equivalent to doing the second style,
and we can use `super()` directly.

So, yeah. Yet another weird thing about py2/py3.
And also add some type hint when easy to do so.
@Exirel
Copy link
Contributor Author

Exirel commented Aug 29, 2021

Fixed merge conflict.

@Exirel Exirel requested a review from dgw August 29, 2021 15:30
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Definitely leaned on GitHub to show me which files were changed since the last review. My little nitpicks are all taken care of, so let's mark this as ready to ship.

@dgw dgw merged commit 125aefe into sopel-irc:master Sep 23, 2021
@Exirel Exirel deleted the class-py3-style branch December 31, 2021 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping Code cleanup, removal of deprecated stuff, etc. Tweak
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants