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

Add addition guides #1178

Merged
merged 6 commits into from
Jan 8, 2019
Merged

Conversation

iamonuwa
Copy link
Contributor

@iamonuwa iamonuwa commented Jan 1, 2019

What was wrong?

Added additional code examples to the docs

Related to Issue #
#1170

How was it fixed?

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

Copy link
Contributor

@stefanmendoza stefanmendoza left a 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

@iamonuwa
Copy link
Contributor Author

iamonuwa commented Jan 2, 2019

@stefanmendoza understood. I'll make changes and update

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.

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.

docs/examples.rst Outdated Show resolved Hide resolved
docs/examples.rst Outdated Show resolved Hide resolved
docs/examples.rst Outdated Show resolved Hide resolved
docs/examples.rst Outdated Show resolved Hide resolved
@iamonuwa
Copy link
Contributor Author

iamonuwa commented Jan 2, 2019

@pipermerriam thanks for the review. I'll update it

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.

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

docs/examples.rst Outdated Show resolved Hide resolved
@iamonuwa
Copy link
Contributor Author

iamonuwa commented Jan 5, 2019

@pipermerriam PTAL.

docs/examples.rst Outdated Show resolved Hide resolved
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.

@iamonuwa I'm sorry for the multiple iterations on this, these should be the last. Good work.

docs/examples.rst Outdated Show resolved Hide resolved
docs/examples.rst Outdated Show resolved Hide resolved
docs/examples.rst Outdated Show resolved Hide resolved
docs/examples.rst Outdated Show resolved Hide resolved
docs/examples.rst Outdated Show resolved Hide resolved
@pipermerriam
Copy link
Member

@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.

@pipermerriam pipermerriam merged commit 4185736 into ethereum:master Jan 8, 2019
@iamonuwa iamonuwa deleted the example-docs-update branch June 18, 2019 18:38
@iamonuwa iamonuwa restored the example-docs-update branch June 18, 2019 18:38
@iamonuwa iamonuwa deleted the example-docs-update branch June 18, 2019 18:38
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.

3 participants