-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add addition guides #1178
Conversation
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 |
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 |
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. |
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