Is the ContractEvent type bugged?

[carver]

What was wrong?

When calling contract.events.Event(), mypy warns that ContractEvent is not callable.

How can it be fixed?

After digging a little deeper, it seems that these three annotations should probably be Type[ContractEvent] instead:

web3.py/web3/contract.py

Lines 246 to 263 in 92cf641

	def __getattr__(self, event_name: str) -> "ContractEvent":
	if '_events' not in self.__dict__:
	raise NoABIEventsFound(
	"The abi for this contract contains no event definitions. ",
	"Are you sure you provided the correct contract abi?"
	)
	elif event_name not in self.__dict__['_events']:
	raise ABIEventFunctionNotFound(
	"The event '{}' was not found in this contract's abi. ".format(event_name),
	"Are you sure you provided the correct contract abi?"
	)
	else:
	return super().__getattribute__(event_name)
	def __getitem__(self, event_name: str) -> "ContractEvent":
	return getattr(self, event_name)
	def __iter__(self) -> Iterable["ContractEvent"]:

[wolovim]

Confirmed the suggested fix addresses the issue. Will get a PR going.

1. Is this something that needs test coverage? If so, any examples you can point to as a template?
2. If test coverage can't be used to detect this sort of thing, any ideas for detecting/preventing this in the future?
3. Since contract functions are typed in what looks to be the same pattern, its not immediately clear to me why they don't throw a similar warning. (Should the 'ContractFunction''s also be updated to Type[ContractFunction]?)

[wolovim]

closed via #1651

[carver]

1. Is this something that needs test coverage? If so, any examples you can point to as a template?
   Hm, there are some places that do explicit mypy checks. Let's see...

• eth-utils makes mypy run on a test file that calls the API: https://github.com/ethereum/eth-utils/blob/386863e6b7d95ac5acb11f6ba81619dc88ed6eb9/tox.ini#L52
• Here's another approach: https://github.com/ethereum/eth-utils/blob/970aba7ac51edb171988200fd2b74e1b36e9615f/tests/functional-utils/type_inference_tests.py
• Maybe @cburgdorf has seen a third approach somewhere? I thought there was another approach, but I'm not finding it

2. If test coverage can't be used to detect this sort of thing, any ideas for detecting/preventing this in the future?

Nothing really besides finding a way to test it.

3. Since contract functions are typed in what looks to be the same pattern, its not immediately clear to me why they don't throw a similar warning. (Should the 'ContractFunction''s also be updated to Type[ContractFunction]?)

It's totally possible that this is broken too, yes. I suspect you're right!
I only noticed this event issue because of a mypy failure in trinity's eth2 code.
I think it will be especially difficult to test it, because you can do everything (type-wise) to the instance as you can to the class. (Since there's a __call__ with Any params, there won't be a problem with init). It might be tough enough and inconsequential enough that it's not worth testing here.

[wolovim]

(Sorry if this was closed prematurely.)

re: 1, The eth-utils examples are interesting. Will explore this more when I get a chance. Would be great to have programmatic confidence that types are accurate.

re: 3, Ah, I did test some ContractFunction functionality, but didn't get any warnings. That'd explain it.

[cburgdorf]

@carver the testing approaches you listed are the only ones I know from the top of my head.

Generally, a high profit gain should also come from working towards strict mypy checking. There is still room in the mypy.ini to tighten some checks (and mypy continues to add stricter checks with new releases) but that's more a general recommendation and I can't say for sure that stricter checks would have caught the bug in question.