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.

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

Use eth-utils methods instead of custom web3 methods #1479

Merged
merged 1 commit into from Nov 4, 2019

Conversation

@kclowes
Copy link
Contributor

kclowes commented Oct 24, 2019

What was wrong?

There were a couple duplicate functions in both eth-utils and web3. We want to standardize on eth-utils.

Related to Issue #1197

How was it fixed?

Took out the duplicate methods from web3, and added a note to the v6 breaking changes for apply_formatters_to_array which is very close but not quite the same between the two libraries.

Todo:

Cute Animal Picture

image

@kclowes kclowes changed the title [WIP] Remove most duplicate methods from eth-utils [WIP] Use eth-utils methods instead of custom web3 methods Oct 24, 2019
@kclowes kclowes force-pushed the kclowes:use-eth-utils branch from 1546a78 to 08e101c Oct 24, 2019
@kclowes kclowes changed the title [WIP] Use eth-utils methods instead of custom web3 methods Use eth-utils methods instead of custom web3 methods Oct 24, 2019
@kclowes kclowes requested a review from pipermerriam Oct 24, 2019
Copy link
Member

pipermerriam left a comment

Since apply_formatters_to_array is under the web3._utils namespace we don't have to maintain backwards compatibility since it's considered a private API (so no need to wait till v6 nor is there need to make the existing formatters importable from web3._utils.formatters

@kclowes

This comment has been minimized.

Copy link
Contributor Author

kclowes commented Oct 25, 2019

@pipermerriam I think it is a breaking change because we use it in middleware? The web3 apply_formatters_to_array converts the result to a list, but the eth-utils version does not. Because we use the web3 apply_formatters_to_array in the pythonic formatting middleware, a bunch of the ENS tests started failing because they were expecting a list to be returned from w3.eth.accounts, but got a tuple instead. I suspect others may run into the same problem. Maybe it's better to use the eth-utils apply_formatters_to_array and convert to a list on the web3 side?

@pipermerriam

This comment has been minimized.

Copy link
Member

pipermerriam commented Oct 25, 2019

Maybe it's better to use the eth-utils apply_formatters_to_array and convert to a list on the web3 side?

That is what I would advocate for (and ideally leave a note to do any cleanup in v6 if you think it's worth removing the shim then)

@kclowes kclowes force-pushed the kclowes:use-eth-utils branch from 1e835bf to 25954b6 Oct 25, 2019
@kclowes kclowes force-pushed the kclowes:use-eth-utils branch from 25954b6 to aca603b Oct 25, 2019
@kclowes kclowes merged commit e086cf2 into ethereum:master Nov 4, 2019
29 checks passed
29 checks passed
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: py36-core Your tests passed on CircleCI!
Details
ci/circleci: py36-ens Your tests passed on CircleCI!
Details
ci/circleci: py36-ethpm Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-ethtester-pyevm Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-http-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-http-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ipc-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ipc-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ws-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ws-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-parity-http Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-parity-ipc Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-parity-ws Your tests passed on CircleCI!
Details
ci/circleci: py37-core Your tests passed on CircleCI!
Details
ci/circleci: py37-ens Your tests passed on CircleCI!
Details
ci/circleci: py37-ethpm Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-ethtester-pyevm Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-http-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-http-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ipc-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ipc-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ws-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ws-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-parity-http Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-parity-ipc Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-parity-ws Your tests passed on CircleCI!
Details
continuous-documentation/read-the-docs Read the Docs build succeeded!
Details
@kclowes kclowes deleted the kclowes:use-eth-utils branch Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.