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.toolz aliasing for cytoolz/toolz #138

Merged
merged 3 commits into from
Oct 16, 2018
Merged

Use eth_utils.toolz aliasing for cytoolz/toolz #138

merged 3 commits into from
Oct 16, 2018

Conversation

stefanmendoza
Copy link
Contributor

@stefanmendoza stefanmendoza commented Oct 16, 2018

What was wrong?

Fixes #111

How was it fixed?

Replaced all the cytoolz imports with eth_tools.toolz. Didn't see any non-cytoolz (i.e. toolz) imports.

Cute Animal Picture

Cute animal picture

@@ -66,7 +66,7 @@ pip install eth-tester
## Development

```sh
pip install -e . -r requirements-dev.txt
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[08:48:27 PM] @stefanmendoza ➜  eth-tester git:(master) python3 -m venv venv
[08:48:33 PM] @stefanmendoza ➜  eth-tester git:(master) source venv/bin/activate
(venv) [08:48:37 PM] @stefanmendoza ➜  eth-tester git:(master) git last
commit fb6d5b9a4e498c2c7a581722d9cc7f24a948f200 (HEAD -> master, tag: v0.1.0-beta.33, upstream/master, origin/master, origin/HEAD)
Author: Jason Carver <[email protected]>
Date:   Thu Oct 4 13:24:39 2018 -0700

    Bump version: 0.1.0-beta.32 → 0.1.0-beta.33
(venv) [08:48:38 PM] @stefanmendoza ➜  eth-tester git:(master) pip install -e . -r requirements-dev.txt           
Could not open requirements file: [Errno 2] No such file or directory: 'requirements-dev.txt'

setup.py Outdated
"toolz>0.8.2,<1;implementation_name=='pypy'",
"cytoolz>=0.8.2,<1.0.0;implementation_name=='cpython'",
install_requires=[
"eth-abi>=1.0.0,<2",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed allowing betas as 1.0.0 came out:
https://github.com/ethereum/eth-abi/releases

Still getting this

eth-utils 1.2.2 has requirement eth-typing<2.0.0,>=1.3.0, but you'll have eth-typing 2.0.0 which is incompatible.
py-evm 0.2.0a33 has requirement eth-typing<2.0.0,>=1.1.0, but you'll have eth-typing 2.0.0 which is incompatible.

due to the eth-abi mention here.

Copy link
Member

Choose a reason for hiding this comment

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

@stefanmendoza if you look through the full pip install logs there should be a line which indicates where/why eth-typing==2.0.0 was chosen.

If that doesn't lead to a fix, the ultimate fix for this is to upgrade both eth-utils and py-evm to newer versions which support eth-typing (@carver we may need to cut a new py-evm beta for that since the newer eth-typing support was just recently merged).

Copy link
Contributor Author

@stefanmendoza stefanmendoza Oct 16, 2018

Choose a reason for hiding this comment

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

I'm kind of confused by whatcha mean?

I think it's coming from eth-abi because in the dependencies (gist here), eth-abi is the only thing that allows eth-typing-2.x:

eth-tester==0.1.0b33
  - eth-abi [required: >=1.0.0-beta.1r,<2, installed: 1.2.0]
    - eth-typing [required: <=2, installed: 2.0.0]

I ran into a similar situation in ethereum/web3.py#1056 (comment), but realized @carver had already fixed this in 1.x and 2.x - patch versions just haven't been released yet. Once eth-abi-1.2.1 is released, this should be fixed automatically by pulling in the latest version on pip install, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll get some eth-abi releases out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done: v1.2.1 and v2.0.0-beta.2

@stefanmendoza
Copy link
Contributor Author

@carver / @pipermerriam ready for review!

],
extras_require=extras_require,
python_requires='>=3.5.3,<4',
Copy link
Contributor Author

@stefanmendoza stefanmendoza Oct 16, 2018

Choose a reason for hiding this comment

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

Restricted to >=3.5.3 because of the following issues:

ethereum/web3.py#1091
python/typing#259 (comment)

Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

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

Generally looks good but we should figure out and fix the eth-typing issue.

@stefanmendoza try updating eth-utils to the latest release, I have a hunch...

setup.py Outdated
"toolz>0.8.2,<1;implementation_name=='pypy'",
"cytoolz>=0.8.2,<1.0.0;implementation_name=='cpython'",
install_requires=[
"eth-abi>=1.0.0,<2",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll get some eth-abi releases out.

@stefanmendoza
Copy link
Contributor Author

Installing collected packages: eth-typing, six, parsimonious, toolz, cytoolz, pycryptodome, eth-hash, eth-utils, eth-abi, eth-keys, rlp, semantic-version, bumpversion, py, virtualenv, pluggy, tox, more-itertools, attrs, atomicwrites, pytest, apipkg, execnet, pytest-forked, pytest-xdist, lru-dict, eth-bloom, py-ecc, pyethash, trie, pycparser, cffi, idna, asn1crypto, cryptography, py-evm, mccabe, pycodestyle, pyflakes, flake8, eth-tester
  Running setup.py develop for eth-tester
Successfully installed apipkg-1.5 asn1crypto-0.24.0 atomicwrites-1.2.1 attrs-18.2.0 bumpversion-0.5.3 cffi-1.11.5 cryptography-2.3.1 cytoolz-0.9.0.1 eth-abi-1.2.1 eth-bloom-1.0.1 eth-hash-0.2.0 eth-keys-0.2.0b3 eth-tester eth-typing-1.3.0 eth-utils-1.2.2 execnet-1.5.0 flake8-3.5.0 idna-2.7 lru-dict-1.1.6 mccabe-0.6.1 more-itertools-4.3.0 parsimonious-0.8.0 pluggy-0.8.0 py-1.7.0 py-ecc-1.4.3 py-evm-0.2.0a33 pycodestyle-2.3.1 pycparser-2.19 pycryptodome-3.6.6 pyethash-0.1.27 pyflakes-1.6.0 pytest-3.9.1 pytest-forked-0.2 pytest-xdist-1.23.2 rlp-1.0.3 semantic-version-2.6.0 six-1.11.0 toolz-0.9.0 tox-2.9.1 trie-1.3.8 virtualenv-16.0.0
tox -elint
lint develop-inst-nodeps: /Users/sm045474/Desktop/eth-tester
lint installed: asn1crypto==0.24.0,cffi==1.11.5,coincurve==9.0.0,cytoolz==0.9.0.1,eth-abi==1.2.1,eth-hash==0.2.0,eth-keys==0.2.0b3,-e git+https://github.com/stefanmendoza/eth-tester.git@4871d4f588fc95c6623b91c800c2d571596baec1#egg=eth_tester,eth-typing==1.3.0,eth-utils==1.2.2,flake8==3.5.0,mccabe==0.6.1,parsimonious==0.8.0,pycodestyle==2.3.1,pycparser==2.19,pyflakes==1.6.0,rlp==1.0.3,semantic-version==2.6.0,six==1.11.0,toolz==0.9.0
lint runtests: PYTHONHASHSEED='1952152132'
lint runtests: commands[0] | flake8 /Users/sm045474/Desktop/eth-tester/eth_tester /Users/sm045474/Desktop/eth-tester/tests
/Users/sm045474/Desktop/eth-tester/.tox/lint/lib/python3.7/site-packages/pycodestyle.py:113: FutureWarning: Possible nested set at position 1
  EXTRANEOUS_WHITESPACE_REGEX = re.compile(r'[[({] | []}),;:]')
_________________________________________________________________________________________________ summary __________________________________________________________________________________________________
  lint: commands succeeded
  congratulations :)

Looks good locally 👍

setup.py Outdated
@@ -50,15 +50,14 @@
url='https://github.com/ethereum/eth-tester',
include_package_data=True,
install_requires=[
"toolz>0.8.2,<1;implementation_name=='pypy'",
"cytoolz>=0.8.2,<1.0.0;implementation_name=='cpython'",
"eth-abi>=1.0.0,<2",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@carver do we want to bump the minimum to 1.2.1? Also, we probably need to rerun the last build to pick up the new release.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think that's fine 👍 -- I tried rerunning, but it still didn't pick up 1.2.1 (until I ssh in, and then ~/.local/bin/tox -elint runs fine.

Anyway, changing the eth-abi req to>=1.2.1,<2 will trigger it again cleanly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed: 23bd953

@carver carver merged commit 8719326 into ethereum:master Oct 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix toolz and cytoolz code
3 participants