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

Type hints for 'return self' #1601

Merged
merged 6 commits into from
Aug 31, 2024
Merged

Type hints for 'return self' #1601

merged 6 commits into from
Aug 31, 2024

Conversation

e10e3
Copy link
Contributor

@e10e3 e10e3 commented Aug 28, 2024

This is an attempt at #1417.
Some commits have already been done about this, but a few uses were left in the code.

I added type hints for the learn_one, learn_many, update, and revert methods in the base classes, meaning MyPy can now be used to detect child classes with the wrong return type.

I believe the only update method with a non-None return is the one in LazySearch, I think the boolean is voluntary and has a use case.

Since the original discussion was about update-like methods in general, I also updated a few methods like _fit where it didn't break the logic.

e10e3 added 4 commits August 28, 2024 11:01
Changing the return types in the base classes means MyPy can detect when
a method returns an unexpected value.
@e10e3
Copy link
Contributor Author

e10e3 commented Aug 28, 2024

I saw that SortedWindow.append has a return self as well, but I ultimately decided against changing it in fear of breaking something, even though it could fit the mould.
Please tell me if it should be changed as well.

@MaxHalford
Copy link
Member

I saw that SortedWindow.append has a return self as well, but I ultimately decided against changing it in fear of breaking something, even though it could fit the mould.

You can change it. In theory it shouldn't return self. We can fix downstream issues if they arise.

@e10e3 e10e3 changed the title Return self Type hints for 'return self' Aug 29, 2024
@MaxHalford MaxHalford merged commit 25f9f89 into online-ml:main Aug 31, 2024
4 checks passed
@e10e3 e10e3 deleted the return-self branch August 31, 2024 19:17
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 this pull request may close these issues.

2 participants