-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add addition guides #1178
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
Add addition guides #1178
Conversation
stefanmendoza
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just an idea, but would it be better to just have the following in the example
from web3.auto import w3
import json
# Example call...and have it use the default provider instead of assuming Infura and having to add an example key?
See: https://web3py.readthedocs.io/en/stable/providers.html#automatic-provider-detection
|
@stefanmendoza understood. I'll make changes and update |
pipermerriam
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great start. One thing I'd like to see is this to be doctested more thoroughly as well as removing some of the duplication in the process.
1: add a setup
You can use the .. testsetup directive to do some pre-setup that won't be rendered in the final docs to setup the w3 instance and/or parse the ABI.
99e14b3#diff-0c4e706274c9fa7b8006416f2043fb46R187
2: change to use doctest
All of the .. code-block:: python block can be converted to `.. doctest::`` directives:
99e14b3#diff-0c4e706274c9fa7b8006416f2043fb46R149
3: Convert to REPL style code blocks
If the code blocks are done as follows (as if the code was being typed into a python REPL, then the code will doctest a bit better as well as allowing you to actually test the print statements.
.. code-block:: python
import sys
print(sys.version)
This would become:
.. doctest
>>> import sys
>>> print(sys.version)
'3.6.5 (default, Aug 27 2018, 12:56:57) \n[GCC 7.3.0]'
The result is that in our CI environment the code blocks will be run and confirmed against their outupt.
4: switch to use the eth-tester backed provider
Instead of using Infura which ends up querying live data, we should instead use the EthereumTesterProvider
from web3 import Web3
w3 = Web3(Web3.EthereumTesterProvider())This will give you an in-memory web3 instance. You'll have to do the work in the .. testsetup directives to create and deploy the contracts you interact with.
|
@pipermerriam thanks for the review. I'll update it |
pipermerriam
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to get rid of the infura usage. See my comment on how to do this. Also, you can test these locally using tox -e doctest. Currently there are a number of failures you can see here: https://circleci.com/gh/ethereum/web3.py/41261?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link
|
@pipermerriam PTAL. |
pipermerriam
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iamonuwa I'm sorry for the multiple iterations on this, these should be the last. Good work.
6f1c74d to
28b628d
Compare
|
@iamonuwa I ended up re-working a lot of this to remove any use of infura from the docs and to clean up your examples a bit. Just wanted to point it out to you so you can see it with a slightly cleaner structure. |
What was wrong?
Added additional code examples to the docs
Related to Issue #
#1170
How was it fixed?
Cute Animal Picture