Web3.py: Deprecation warnings not raised inside a @combomethod

[carver]

What was wrong?

@veox discovered that deprecation warnings via @deprecated_for are not correctly raised when combined with the @combomethod decorator.

How can it be fixed?

:man_shrugging:

[veox]

For ref, @combomethod is for both of the following to work:

# call as object/instance method
In [1]: from web3.auto import w3

In [2]: w3.soliditySha3(['bool'], [True])
Out[2]: HexBytes('0x5fe7f977e71dba2ea1a68e21057beebb9be2ac30c6410aa38d4f3fbe41dcffd2')

# call as class method
In [3]: import web3

In [4]: web3.Web3.soliditySha3(['bool'], [True])
Out[4]: HexBytes('0x5fe7f977e71dba2ea1a68e21057beebb9be2ac30c6410aa38d4f3fbe41dcffd2')

Note how neither produces a DeprecationWarning. (The differentiation between solidityKeccak() and soliditySha3() was made in PR #1139.)

For comparison, sha3 (instead of soliditySha3):

In [1]: from web3.auto import w3

In [2]: w3.sha3(0)
/usr/bin/ipython:1: DeprecationWarning: sha3 is deprecated in favor of keccak
  #!/usr/bin/python3
Out[2]: HexBytes('0xbc36789e7a1e281436464229828f817d6612f7b477d66591ff96a9e064bcc98a')

In [3]: import web3

In [4]: web3.Web3.sha3(0)
/usr/bin/ipython:1: DeprecationWarning: sha3 is deprecated in favor of keccak
  #!/usr/bin/python3
Out[4]: HexBytes('0xbc36789e7a1e281436464229828f817d6612f7b477d66591ff96a9e064bcc98a')

Guess: this works because sha3 is just a @staticmethod, which is a _built-in_ Python decorator.

---

In web3.py, @combomethod was introduced in commit 74dd65902f56fdc5a3062b340fdfc2506b65808a, and has been mostly unchanged since; found at

https://github.com/ethereum/web3.py/blob/0cbb1e4d3bf5f5fe1e7493f374d824b112d389d8/web3/_utils/decorators.py#L6-L17

---

There are contract.* functions (listed in https://github.com/ethereum/web3.py/issues/722#issuecomment-376617343), e.g:

https://github.com/ethereum/web3.py/blob/0cbb1e4d3bf5f5fe1e7493f374d824b112d389d8/web3/contract.py#L276-L278

... where this would be most severe, since the code between "deprecated" and "recommended" can differ (i.e. the former is not always a simple wrapper around the latter).

[davesque]

The cause of the issue is explained here:
https://docs.python.org/3.7/library/warnings.html#default-warning-filter

When you override the warning category to DeprecationWarning (as is done here), you become subject to the default warning filter which filters out warnings that target developers (like deprecation warnings) unless that warning is "triggered directly by code in __main__." As it turns out, "triggered directly by code in __main__" actually means "triggered at the stack level of __main__."

As far as @veox 's examples go, when there are two decorators, the __main__ module's stack level is 3 relative to the warning statement. This is because the use of decorators involves wrapping the call site of the decorated function inside a wrapping function which leads to the creation of an additional stack frame. When there is one decorator (including any built-in decorators, which don't create any stack frames since they're implemented in C), the __main__ module's stack level is 2 relative to the warning statement and the warning counts as having been triggered in __main__ (because of this) so it doesn't get filtered.

I think there are a couple of ways to fix this. One would be to remove the category override and just have our deprecation warnings count as user warnings. Then they won't get filtered by default. I'm not sure I like that though since I understand the logic behind the default filtering behavior. The other option is to advise people somehow to use the PYTHONWARNINGS=default environment variable setting in development or when running test suites and such.

[kclowes]

Yeah, in thinking about this a little bit more, I agree with leaving it and encouraging people to use the PYTHONWARNINGS=default. I'll add a note to the docs. I was thinking maybe the top of the Release Notes section, but if anyone has opinions on that, let me know. I do think it's a little weird that other deprecation warnings show, and only the ones inside the combomethod decorator do not, but as far as I can tell, they're all getting removed for v5 anyway.