Web3.py: Python Modernization

[carver]

What was wrong?

A lot of code cruft exists to support versions of Python before 3.5. It complicates the implementation, and requires more time to test.

How can it be fixed?

In v4, as part of dropping support for Python 2 and Python <=3.4, we can remove this cruft. Also, let's drop test environments for python 2.7 and 3.4, and add one for 3.6.

• [x] Remove Python 2.7 and 3.4 tests #349
• [x] Add Python 3.6 test environment
• [x] Remove all sys.version_info checks, presuming Python 3.5+
• [x] Remove all bytes == str checks, presuming Python 3.5+
• [x] Remove the web3.utils.compat module, and replace all usages with native calls #251
• [ ] Bonus: Convert logic, where appropriate, to use native methods in 3.5+ (like b'\xbe\xef'.hex(), etc.)

[gitcoinbot]

__This issue now has a funding of 0.95 ETH (313.05 USDT) attached to it.__ To view or claim this funding, click here.

[jasonrhaas]

Just submitted a claim on this bounty through gitcoin.co! I did it through the Google Form since I was having a bit of trouble with MetaMask.

[owocki]

@jasonrhaas i got your claim. i'm confused though. it looks like @techtonik is the one who submitted the actual code?

[jasonrhaas]

@owocki I was under the impression that there was still work that needed to be done since not all of the checkmarks were marked complete and the claim was still open.

[owocki]

ah yes, you are correct.

[jasonrhaas]

@owocki Am I good to start working on this issue for the bounty? I'm new to the whole gitcoin.co thing, first time trying it out.

[owocki]

please do @jasonrhaas !

[pipermerriam]

@jasonrhaas assuming @techtonik is also interested in this (since he has already submitted one pull request on the subject) it's likely we'll look at dividing the bounty between the two of you (also assuming that @techtonik is interested in collecting the bounty). Exact division TBD based on something like the portion of the work done by the two of you.

Wanted to be upfront about expectations.

[jasonrhaas]

@pipermerriam Fine with me, I'll leave it up to you to decide how the bounty gets split based on work put in.

[jasonrhaas]

So I'm having an issue getting the tests to run (without any code change). It seems like there is an issue with geth.get_gethversion(). When running 'pytest -n 4 -f --maxfail=1' it fails here:

[gw0] ERROR tests/integration/test_goethereum.py::TestGoEthereum::test_soliditySha3[types2-values2-0\xf9]!\x07\x85`\x1e\xb3:\xe4\xd5=@[&\xf9 \xe7e\xdf\xf8|\xca\x8e\x9aJ\xec\x99\xf8&q] <- web3/utils/module_testing/web3_module.py

So I dug into this a bit and noticed that when I import geth and try running the get_geth_version() method I get the same error.

>>> geth.get_geth_version()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.6/site-packages/geth/main.py", line 28, in get_geth_version
    version_info_string = get_geth_version_info_string(**geth_kwargs)
  File "/usr/local/lib/python3.6/site-packages/geth/main.py", line 20, in get_geth_version_info_string
    stdoutdata, stderrdata, command, proc = geth_wrapper(**geth_kwargs)
  File "/usr/local/lib/python3.6/site-packages/geth/wrapper.py", line 262, in geth_wrapper
    stderr_data=stderrdata,
geth.exceptions.GethError: An error occurred during execution
> command: `nice -n 20 geth version`
> return code: `127`
> stderr:

> stdout:
nice: geth: No such file or directory

Seems like its trying run an OS command, but the geth command is not available in the environment.

[pipermerriam]

Yes, you'll need go-ethereum installed on your system to run that part of the test suite.

You can run the important subset of the test suite by running py.test tests/core which won't require go-ethereum.

Also, you can run a subset of the integration tests by running py.test tests -k "not goethereum"

[zheli]

What else is left on this bounty?

[carver]

@zheli chances are good that the two people who started in on this will complete everything. The python 3.6 testing is basically done in #438, but other than that, anything unchecked above is still not fully complete.

[jasonrhaas]

Yes this one should be complete today.

[zheli]

@carver @jasonrhaas thx. I will find something else then 👍

[owocki]

@zheli (i saw your comment on another gitcoin funded issue, so i assume you are funding these issues view gitcoin). here are some other python issues -- https://gitcoin.co/explorer?q=python

feel free to reach out on slack (https://gitcoin.co/slack) or gitter https://gitter.im/gitcoinco and we'll find you something

[techtonik]

Yep. I came here for the bounty, but cleaning up the code takes more time than I expected. :D

I don't mind splitting the bounty. In fact, I want to see how it would play to test this collaborative model, @owocki. I could not understand if I need to claim issue before doing the work or after. It seemed wrong to me to claim issue that may become too epic for me to complete.

[owocki]

word. @techtonik -- you don't need to claim the issue until its ready for payout.

[zheli]

@owocki @techtonik maybe should change the word "claim" to something else to make it clear? Like "apply for payout" or something...

[jasonrhaas]

What else needs to be done to finish up this issue and claim the bounty? PR has been merged in. If there is still work left to be done let me know and I'll get it finished up so we can close this one.

[owocki]

looks to me like theres still a few unchecked ticks on the issue PR? if those are done, you could tag piper to give his 👍

[jasonrhaas]

@pipermerriam @carver anything else that needs to be done on this one to complete? Are we good after PR #438 ?

[miguel550]

@jasonrhaas I just searched for "sys.version_info" in this repo after PR #438 merged and it still shows version 2 compatibility code.

[carver]

@jasonrhaas - yes @miguel550 is correct there are still sys.version_info lines all throughout the code that need to be removed

[jasonrhaas]

I’m on it, PR coming shorty.

Sent from my portal gun.

On Nov 21, 2017, at 6:23 PM, Jason Carver [e-mail] wrote:

@jasonrhaas - yes @miguel550 is correct there are still sys.version_info lines all throughout the code that need to be removed

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[techtonik]

@owocki @techtonik maybe should change the word "claim" to something else to make it clear? Like "apply for payout" or something...

"checkout"?

[owocki]

@jasonrhaas once that PR is in, let me know and if it's ok with the web3.py folks ill payout the bounty

[jasonrhaas]

Ok I have all the sys.version_info removed. Looking for the bytes == str checks now...

[jasonrhaas]

hey @carver what do you mean by the bytes == str checks? I found one line that was doing a if bytes is str: which returns False for Python 3 and True for Python 2. I deleted that check. The rest of the stuff was using the sys.version_info method which has all been removed.

[carver]

@jasonrhaas Yup, I thought there were maybe a few places with bytes is str, but now I don't see more than that one either. Looks good to me.

[owocki]

👌 to payout?

[carver]

We have to split up the bounty, because several people did work on it. Lines of code is usually a terrible way to judge effort, but on this particular issue, it seems like an okay proxy.

@jasonrhaas did:

438 +35 -137

471 +29 -50

Total: 251

@miguel550 did:

456 +34 -171

Total: 205

@techtonik did:

434 +5 -29

Total: 34

All LOC changes: 490

Payout percentages:
@techtonik 7%
@miguel550 42%
@jasonrhaas 51%

Did I miss any contributions?

[jasonrhaas]

Fine with me. Definitely an experiment on how to deal with multiple people working on one bounty. Lines of code is OK in this case I agree... but normally that wouldn't work so well.

[owocki]

We have to split up the bounty, because several people did work on it. Lines of code is usually a terrible way to judge effort, but on this particular issue, it seems like an okay proxy.

Did I miss any contributions?

@jasonrhaas @miguel550 @techtonik thanks for your work on this. im going to fulfilled the bounty to myself and then remit the above payments as tips. Soon, we are going to integrate StandardBounties which will more formally support payouts to multiple parties.

i will not take action on the above plan for the next 12 hours just in case anyone has concerns or we missed any contributions. please reach out to me on gitcoin.co/slack if any problems.

[owocki]

have not heard anything from anyone. payout imminent (est in ahour or two)

[miguel550]

@owocki I haven't received anything yet. Did the payout happened?

[owocki]

sry. been distracted with _all the things_ on it now

[gitcoinbot]

⚡️ A tip worth 0.51 ETH has been granted to @jasonrhaas for this issue. ⚡️

Nice work @jasonrhaas, check your email for further instructions. | Send a Tip

[gitcoinbot]

⚡️ A tip worth 0.42 ETH has been granted to @miguel550 for this issue. ⚡️

Nice work @miguel550, check your email for further instructions. | Send a Tip

[gitcoinbot]

⚡️ A tip worth 0.07 ETH has been granted to @techtonik for this issue. ⚡️

Nice work @techtonik, check your email for further instructions. | Send a Tip

[carver]

Looks like everything was paid out, closing...

[owocki]

ok jsut sent all 3 tips. @techtonik @miguel550 @jasonrhaas you should receive an email with instructions on how to redeem to an address you own.

sometimes github doesnt return an email address for certain profiles. if you dont get an email send me an email (or message on the gitcoin slack) with your github username and email address and i will resend it

[carver]

Thanks @techtonik @miguel550 @jasonrhaas for your help, and @owocki for the sponsorship!

[owocki]

btw, the OG bounty was for 0.95, but i tip'd you all a % of 1 ETH because you've been good sports. also i'm lazy and didnt want to do the * 0.95 math 😂

[jasonrhaas]

got it, thanks guys. looking forward to contributing more 👍

[techtonik]

@owocki thanks! I don't know what to do with that ETH yet.

I wonder if it is also possible to count the complexity of the issue in ETH as well? I'd like to detect underfunded issues. I understand that funds are limited and bounties are bounties, but it would be awesome to see real cost of the issues. Perhaps in lifetime hours, because USD measure is too unequal.

[owocki]

@techtonik you'd be fine just HODLing it im sure, but https://www.cryptokitties.co is the latest app people are excited about. YMMV

there's a bunch of thoughts on https://github.com/gitcoinco/web/issues/21 and https://github.com/gitcoinco/web/issues/22 with respect to pricing and estimation of issues. i like your idea about measuring in hours, but the pragmatist in me knows its very hard to do when you have a talented team doing estimates, much less doing it alogirthmicly!

hit me back on either of those issues or on gitcoin slack and lets chat more about this :)