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.

Sign up for GitHub

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

Jump to bottom

Pacify Python DeprecationWarnings + add solidityKeccak() and deprecate soliditySha3() #1139

Merged
merged 4 commits into from
Dec 13, 2018
Merged

Pacify Python DeprecationWarnings + add solidityKeccak() and deprecate soliditySha3() #1139

merged 4 commits into from
Dec 13, 2018

Conversation

veox
Copy link
Contributor

@veox veox commented Nov 16, 2018 •
edited
Loading

What was wrong?

Without export PYTHONWARNINGS=ignore, many programs are now overly noisy.

How was it fixed?

Collecting and squashing these warnings here.

Snippet for releases.rst

* `Web3.soliditySha3` has been deprecated; use `Web3.solidityKeccak` instead.

Cute Animal Picture

"Whistle if you see 'em!.."

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

Source: Wikipedia; by April King

@veox veox changed the title [WIP] Squash DeprecationWarnings [WIP] Pacify Python DeprecationWarnings Nov 16, 2018
@veox veox force-pushed the squash-deprecation-warnings branch from 3de63f5 to 81f027a Compare November 16, 2018 21:12
@veox veox force-pushed the squash-deprecation-warnings branch from 81f027a to 9644051 Compare November 16, 2018 21:16
@veox
Copy link
Contributor Author

veox commented Nov 16, 2018

Note: we can also help other projects with theirs:

@veox veox force-pushed the squash-deprecation-warnings branch 4 times, most recently from 1a6aa81 to 15a7830 Compare November 16, 2018 23:47
@carver
Copy link
Collaborator

carver commented Nov 17, 2018

Looking good! Still a WIP?

Are you up for opening another PR against the v4 branch, too?

@veox veox mentioned this pull request Nov 17, 2018
Closed
@veox
Copy link
Contributor Author

veox commented Nov 17, 2018 •
edited
Loading

Looking good! Still a WIP?

Yes, still a WIP: I'd like to run the full test suite locally, in one go, so the DeprecationWarnings are all in one place. See if I missed anything.

Are you up for opening another PR against the v4 branch, too?

I'll go ahead and try cherry-picking.

@veox veox force-pushed the squash-deprecation-warnings branch from 15a7830 to f420eb2 Compare November 17, 2018 13:15
@veox
Copy link
Contributor Author

veox commented Nov 17, 2018

v4 and master have diverged too much for my liking to work on it on a Saturday (or ever). I won't be doing the v4 part, sorry.

@veox
Copy link
Contributor Author

veox commented Nov 17, 2018 •
edited
Loading

The remaining warnings are:

  • (mostly) the websockets one - see upstream;
  • an expected one for test_eth_getTransactionFromBlockHashAndIndex (a deprecated method with an available updated counter-part, that is still tested);
  • a ResourceWarning from asyncio, which I believe comes from lahja, and is out of scope for this PR.

IMO ready, unmarking [WIP].

@veox veox changed the title [WIP] Pacify Python DeprecationWarnings Pacify Python DeprecationWarnings Nov 17, 2018
veox
veox commented Nov 17, 2018
View reviewed changes
web3/main.py Outdated Show resolved Hide resolved
@veox
Pacify Python warnings: sha3 has been renamed to keccak.
9cd9918 
SQUASHED:

main: minor, fix incorrect deprecation message.

Reads as

    sha3 is deprecated in favor of This method has been renamed to keccak

otherwise.
@veox veox force-pushed the squash-deprecation-warnings branch from f420eb2 to 9cd9918 Compare November 17, 2018 13:49
veox
veox commented Nov 17, 2018
View reviewed changes
web3/main.py
@@ -173,9 +173,15 @@ def keccak(primitive=None, text=None, hexstr=None):
)

@combomethod
@deprecated_for("solidityKeccak")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deprecation message is "swallowed" (never actually shown); see issue #1140.

@pipermerriam
Copy link
Member

I think this should also be backported to v4

@carver carver mentioned this pull request Nov 19, 2018
Closed
@carver
Copy link
Collaborator

carver commented Nov 19, 2018

I think this should also be backported to v4

Yup. I opened a new issue since veox already said he wasn't into it: #1143

carver
carver requested changes Nov 19, 2018
View reviewed changes
web3/main.py Outdated Show resolved Hide resolved
@veox veox changed the title Pacify Python DeprecationWarnings Pacify Python DeprecationWarnings + add solidityKeccak() and deprecate soliditySha3() Dec 5, 2018
@kclowes kclowes merged commit d96fb53 into ethereum:master Dec 13, 2018
kclowes pushed a commit to pjryan93/web3.py that referenced this pull request Apr 25, 2019
@veox @kclowes
soliditySha3: use passed cls (instance or class) instead of class (…
72dc15c 
…always).

Addresses criticism

ethereum#1139 (review)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pipermerriam pipermerriam pipermerriam approved these changes

@carver carver carver approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@veox @carver @pipermerriam @kclowes