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

Uses Self Type in contract factory methods #2997

Closed
wants to merge 5 commits into from

Conversation

konichuvak
Copy link
Contributor

What was wrong?

  1. Return type of Contract.factory() is erroneously annotated as "Contract" instead of Type["Contract"]

  2. Even once we fix the error above, type annotation for subclasses of Contract are broken

from typing import Type

from web3 import Web3
from web3.contract import Contract


class MyContract(Contract):
    address = Web3.to_checksum_address("0x0000000000000000000000000000000000000000")

    @classmethod
    def factory(cls, *args, **kwargs) -> Type["Contract"]:
        return super().factory(*args, **kwargs)

    def my_method(self):
        pass

contract_factory = MyContract.factory(Web3())()
contract_factory.my_method()  # error: "Contract" has no attribute "my_method"  [attr-defined]

How was it fixed?

Self Type was added in Python 3.11 which solves this problem of underspecified return types of constructor methods for subclasses.

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@pacrob
Copy link
Contributor

pacrob commented Jun 20, 2023

Self is definitely a helpful addition, but we need to maintain compatibility with all active versions of python. 3.7 is being deprecated soon, but it'll be a while before we can use features that are brand new in 3.11.

I'll leave this open in case you want to try a different solution that maintains compatibility.

@konichuvak
Copy link
Contributor Author

Self is definitely a helpful addition, but we need to maintain compatibility with all active versions of python. 3.7 is being deprecated soon, but it'll be a while before we can use features that are brand new in 3.11.

I'll leave this open in case you want to try a different solution that maintains compatibility.

Thanks for reviewing the PR.
I am not sure which part is incompatible with Python3.7 in here? We are importing Self from typing_extensions, which supports 3.7 and above.

@pacrob
Copy link
Contributor

pacrob commented Jun 21, 2023

OK, dug in a bit further. You are correct that we can use Self with earlier python versions, which is great news. It looks like mypy doesn't support it until v1.0.0 though, and when I bump that it opens a number of other typing issues to deal with.

Once we get that in, I'll come back and pull this in. Thank you!

konichuvak added 2 commits June 21, 2023 22:04
1. Uses the recently added `Self` Type for better type inference whenever Contract is subclassed

2. Fixes the return type of `Contract.factory` to indicate that a class is returned instead of an instance thereof
Fixes / ignores any warnings that appeared as a result
@konichuvak
Copy link
Contributor Author

It looks like mypy doesn't support it until v1.0.0 though

Great point, I did not catch that.
I bumped it to >=1.0.0 and fixed / ignored all the typing issues that popped up as a result!
For the exception of the ones in ethpm-spec which I am not sure how to deal with. It seems like that module is only type-checked when when mypy is ran locally.

@pacrob pacrob force-pushed the typing/self branch 2 times, most recently from 241aa75 to 2792dd4 Compare June 23, 2023 21:15
@pacrob
Copy link
Contributor

pacrob commented Jun 23, 2023

ethpm is being deprecated, so probably OK to drop it from mypy checking. I'm not sure why the CI insists on running on your account though - it usually switches to our resources once I push a commit. Maybe try deleting the web3.py project from your CircleCI account?

I've branched it in #3001 and CI is passing there, so we can just use that if we can't get it to run here.

@pacrob
Copy link
Contributor

pacrob commented Jun 27, 2023

Thanks for this, @konichuvak ! Your original commits are merged in #3001

@konichuvak konichuvak deleted the typing/self branch June 27, 2023 20:10
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.

None yet

2 participants