Snakecase contract event methods #2709

linda-le1 · 2022-11-03T06:23:57Z

What was wrong?

Inherited javascript syntax. This PR does not include a deprecation step, intended for v6.
Related to Issue #2598

How was it fixed?

Changed processReceipt to process_receipt, processLog to process_log, createFilter to create_filter in BaseContractEvent. Change getLogs to getlogs in ContractEvent and AsyncContractEvent.

Todo:

Add entry to the release notes

Cute Animal Picture

fselmo · 2022-12-05T18:30:35Z

web3/contract.py

@@ -1681,7 +1681,7 @@ def getLogs(

 class AsyncContractEvent(BaseContractEvent):
    @combomethod
-    async def getLogs(
+    async def get_logs(
        self,
        argument_filters: Optional[Dict[str, Any]] = None,
        fromBlock: Optional[BlockIdentifier] = None,


I think these can be snake cased... all the way up until we get to building the FilterParams typed dict.

fselmo · 2022-12-05T18:53:54Z

web3/types.py

@@ -149,8 +149,8 @@ class FormattersDict(TypedDict, total=False):
 class FilterParams(TypedDict, total=False):
    address: Union[Address, ChecksumAddress, List[Address], List[ChecksumAddress]]
    blockHash: HexBytes
-    from_block: BlockIdentifier
-    to_block: BlockIdentifier
+    fromBlock: BlockIdentifier


I think ultimately we make these changes here to camel case and everything else that we can leave snake cased should probably be snake cased. My reasoning is that the method arguments to python functions are expected to be snake cased, by convention. They are just variables up until we construct this TypedDict. Once we start building the TypedDict, this is the one translation step that basically says "Ok, we passed everything around using python conventions but now we need to speak to a service that does not use python conventions". The way I see it behaving is these TypedDict request param classes are the translation layer between python and provider and I think ultimately this is the only place we need the explicit change to camelCase.

I'm curious though if by doing this we run into complications elsewhere. Thoughts @linda-le1 @kclowes @pacrob?

My approach would be to camel case everything that is in the web3/types.py that is a class making requests to the provider and leave everything else that can be snake cased as snake case.

Yeah, I think I'm in the same boat. Basically anything that goes to/from the node should be camelCase, and everything else should be snaked.

fselmo · 2022-12-05T18:59:49Z

@linda-le1 looks like this needs to be rebased with master as well 👀

docs/contracts.rst

pacrob · 2022-12-05T20:55:33Z

I left one note for the docs anchor title. It also looks like you could remove the Eth.getLogs deprecation notice in docs/web3.eth.rst. Otherwise lookin' good, thanks!

kclowes

Awesome! 🚀 I just left one nit pick. Let me/us know if you have questions about when to snake_case vs. camelCase based on @fselmo's comment.

kclowes · 2022-12-05T18:56:12Z

tests/core/filtering/test_contract_get_logs.py

@@ -19,12 +19,12 @@ def test_contract_getLogs_all(
    txn_hash = contract.functions.logNoArgs(event_id).transact()


I can't comment on the actual line, but looks like the test name on line 10 needs a snake case: test_contract_get_logs_all

kclowes · 2022-12-05T21:00:52Z

web3/types.py

@@ -149,8 +149,8 @@ class FormattersDict(TypedDict, total=False):
 class FilterParams(TypedDict, total=False):
    address: Union[Address, ChecksumAddress, List[Address], List[ChecksumAddress]]
    blockHash: HexBytes
-    from_block: BlockIdentifier
-    to_block: BlockIdentifier
+    fromBlock: BlockIdentifier


Yeah, I think I'm in the same boat. Basically anything that goes to/from the node should be camelCase, and everything else should be snaked.

web3/contract.py

docs/examples.rst

pacrob

lgtm!

kclowes

LGTM too! Thanks @linda-le1!

Edit: looks like we need to clean up some commits. I think it would be clearest to have one commit for each of the snake case-s: getLog -> get_log, processReciept -> process_receipt, createFilter -> create_filter, and the kwargs, but could also see just squashing into one if that's too complicated.

fselmo

lgtm!

linda-le1 · 2022-12-13T06:48:26Z

@kclowes I've consolidated commits - let me know if that looks okay to you. Thanks!

linda-le1 force-pushed the snakecase-contract-event branch from 0d9f493 to cce68a4 Compare November 13, 2022 23:26

linda-le1 force-pushed the snakecase-contract-event branch from cce68a4 to d88c129 Compare November 21, 2022 04:34

linda-le1 force-pushed the snakecase-contract-event branch 2 times, most recently from 7c6f409 to db12062 Compare December 5, 2022 05:38

linda-le1 marked this pull request as ready for review December 5, 2022 05:58

linda-le1 requested review from fselmo, pacrob and kclowes December 5, 2022 06:11

fselmo reviewed Dec 5, 2022

View reviewed changes

pacrob reviewed Dec 5, 2022

View reviewed changes

docs/contracts.rst Outdated Show resolved Hide resolved

pacrob reviewed Dec 5, 2022

View reviewed changes

docs/contracts.rst Outdated Show resolved Hide resolved

kclowes reviewed Dec 5, 2022

View reviewed changes

linda-le1 commented Dec 6, 2022

View reviewed changes

web3/contract.py Show resolved Hide resolved

linda-le1 force-pushed the snakecase-contract-event branch 3 times, most recently from 48670ed to 7c5d04d Compare December 6, 2022 04:47

linda-le1 commented Dec 7, 2022

View reviewed changes

docs/examples.rst Show resolved Hide resolved

pacrob approved these changes Dec 12, 2022

View reviewed changes

kclowes approved these changes Dec 12, 2022

View reviewed changes

fselmo approved these changes Dec 12, 2022

View reviewed changes

linda-le1 force-pushed the snakecase-contract-event branch from 1753aa3 to c1137a4 Compare December 13, 2022 06:42

linda-le1 added 5 commits December 12, 2022 23:49

Snakecase processReceipt

0e83bcb

Snakecase processLog

47487fe

Snakecase createFilter

77bf4a8

Snakecase getLogs

e24e143

Add entry for release notes

38e04a1

linda-le1 force-pushed the snakecase-contract-event branch from c1137a4 to 38e04a1 Compare December 13, 2022 06:49

linda-le1 requested a review from kclowes December 13, 2022 06:58

linda-le1 merged commit e04acaf into ethereum:master Dec 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Snakecase contract event methods #2709

Snakecase contract event methods #2709

linda-le1 commented Nov 3, 2022 •

edited

Loading

fselmo Dec 5, 2022 •

edited

Loading

fselmo Dec 5, 2022 •

edited

Loading

kclowes Dec 5, 2022

fselmo commented Dec 5, 2022

pacrob commented Dec 5, 2022

kclowes left a comment

kclowes Dec 5, 2022

kclowes Dec 5, 2022

pacrob left a comment

kclowes left a comment •

edited

Loading

fselmo left a comment

linda-le1 commented Dec 13, 2022

		@@ -19,12 +19,12 @@ def test_contract_getLogs_all(
		txn_hash = contract.functions.logNoArgs(event_id).transact()

Snakecase contract event methods #2709

Snakecase contract event methods #2709

Conversation

linda-le1 commented Nov 3, 2022 • edited Loading

What was wrong?

How was it fixed?

Todo:

Cute Animal Picture

fselmo Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

fselmo Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

kclowes Dec 5, 2022

Choose a reason for hiding this comment

fselmo commented Dec 5, 2022

pacrob commented Dec 5, 2022

kclowes left a comment

Choose a reason for hiding this comment

kclowes Dec 5, 2022

Choose a reason for hiding this comment

kclowes Dec 5, 2022

Choose a reason for hiding this comment

pacrob left a comment

Choose a reason for hiding this comment

kclowes left a comment • edited Loading

Choose a reason for hiding this comment

fselmo left a comment

Choose a reason for hiding this comment

linda-le1 commented Dec 13, 2022

linda-le1 commented Nov 3, 2022 •

edited

Loading

fselmo Dec 5, 2022 •

edited

Loading

fselmo Dec 5, 2022 •

edited

Loading

kclowes left a comment •

edited

Loading