search

ethereum / eth-typing

Python types for type hinting commonly used ethereum types
MIT License
31 stars 35 forks source link

Deprecation warnings for unused ABI types. #74

Closed reedsa closed 5 months ago

reedsa commented 5 months ago

Deprecated ABIEventComponent, ABIEventParam, ABIFunctionComponent and ABIFunctionParam for removal in v5.

Todo:

Cute Animal Picture

kclowes commented 5 months ago

It would be good to get some more input on the refactor to ABIComponent* before the next release.

I think this pattern makes sense, assuming you're referring to #73? Was there anything specific you were wanting input on?

fselmo commented 5 months ago

I think this pattern makes sense, assuming you're referring to #73? Was there anything specific you were wanting input on?

Yeah changes here and in #73... no reason other than a sanity check before we head down the direction of eliminating some types that have been around for a while. Thanks for eyes!

kclowes commented 5 months ago

Gotcha. Are these types moving from web3 to here? It looks like we could use a @warnings.deprecated decorator on the class if we wanted to publicize that this is getting deprecated via the code: https://peps.python.org/pep-0702/

kclowes commented 5 months ago

Dropping these might be a good thing to do before we stabilize

fselmo commented 5 months ago

I believe dropping all that is in the works atm by @reedsa once these changes are finalized in here. And yeah they are moved here from web3 via #61 #73 and #74 (this PR).

It looks like we could use a @warnings.deprecated decorator on the class if we wanted to publicize that this is getting deprecated via the code: https://peps.python.org/pep-0702/

That's what we were missing. Sgtm as well 👍🏼

reedsa commented 5 months ago

Glad to see the deprecations will likely work, I'll get that in this afternoon! 💯

Agree on the above discussion and those types will be removed later, possibly in a separate PR. I currently have those eliminated in my WIP web3 branch: https://github.com/ethereum/web3.py/pull/3361/files#diff-e46a5be3efb58e124455f1536113138b6e749add824676feda67494f74a15c94L76

reedsa commented 5 months ago

Looked into using @warnings.deprecated and I noticed it's not available until Python >= 3.13. I'll leave the deprecations in the docstrings but let me know if there's anything else needed on this. @kclowes @pacrob @fselmo

fselmo commented 5 months ago

Looked into using @warnings.deprecated and I noticed it's not available until Python >= 3.13. I'll leave the deprecations in the docstrings but let me know if there's anything else needed on this. @kclowes @pacrob @fselmo

Check out the backwards compatibility section in the PEP. It doesn't look like we should have any issues implementing.