Improve docs for abi required-ness #3137
Conversation
docs/web3.eth.rst
Outdated
| @@ -1238,6 +1238,11 @@ Contracts | |||
|
|
|||
| .. py:method:: Eth.contract(address=None, contract_name=None, ContractFactoryClass=Contract, **contract_factory_kwargs) | |||
|
|
|||
| An ``abi`` specifies how a contract can be interacted with. The ``abi`` | |||
There was a problem hiding this comment.
I feel like this clarification should go down below, as in the "following arguments are accepted" section. This first section discusses the actual named arguments associated with the function, in order. abi, while necessary for making contract calls, isn't a named arg, just an optional kwarg (whether this is a good design is perhaps another question 🤔 ) Maybe adding it as a description to the optional kwargs list is a better place? That then opens it as a place for describing usage of the others in that list.
Just my feels, open for discussion!
There was a problem hiding this comment.
Would be ideal to call something out here in the contract docs. I wonder if it would help to include abi as an argument in the method signature above, or some way of indicating it is necessary for the interface to work.
Agree this paragraph doesnt seem quite right in the contract API docs. I think it might clutter the list down below, but there is a link to the Contracts guide from there. I think the paragraph might be a good replacement for the contract docs under Contract.abi. What do you think?
There was a problem hiding this comment.
Agree with putting more info under Contract.abi. Could even explain what ABI stands for there. I also agree with some indication here that abi is required if you want to interact with the contract, maybe just not first thing?
| - ``user_doc`` | ||
|
|
||
| See :doc:`web3.contract` for more information about how to use contracts. | ||
| :param abi: Application Binary Interface. Usually provided since an ``abi`` is required to interact with any contract. |
docs/web3.eth.rst
Outdated
| :param decode_tuples: Optionally convert tuples/structs to named tuples | ||
| :param interface: | ||
| :param metadata: Contract Metadata generated by the compiler | ||
| :param opcodes: Opcodes of the contracts generated by the compiler |
There was a problem hiding this comment.
| :param opcodes: Opcodes of the contracts generated by the compiler | |
| :param opcodes: Opcodes for the contract generated by the compiler |
Is this meant to be singular here?
docs/web3.eth.rst
Outdated
| :param user_doc: | ||
| :return: Instance of the contract | ||
| :rtype: Contract | ||
| :raises TypeError: if the address is not provided |
There was a problem hiding this comment.
| :raises TypeError: if the address is not provided | |
| :raises TypeError: If the address is not provided |
Everything else seems to be capitalized here.
docs/web3.eth.rst
Outdated
| :return: Instance of the contract | ||
| :rtype: Contract | ||
| :raises TypeError: if the address is not provided | ||
| :raises AttributeError: if the contract class is not initialized |
There was a problem hiding this comment.
| :raises AttributeError: if the contract class is not initialized | |
| :raises AttributeError: If the contract class is not initialized |
| @@ -0,0 +1 @@ | |||
| Additional contract ``abi`` documentation to make it a clear requirement for contract instances. | |||
There was a problem hiding this comment.
| Additional contract ``abi`` documentation to make it a clear requirement for contract instances. | |
| Additional contract ``abi`` documentation to make it a clear requirement for interacting with contract instances. |
Perhaps that distinction? Are we allowed to create a contract instance without an ABI? I'm not at my computer at the moment to verify but, if so, is there a particular reason we even allow that and don't validate it as an input to __init__()?
Just curious on improvements that could come in v7 but also maybe we should clarify the exceptions in the docs as well, if they exist.
There was a problem hiding this comment.
I recall an example that used a contract without an abi, I'll dig it up in the docs in a bit. It is possible to create, though very limited in what it can do.
I have no clear idea why it isn't enforced but maybe it is so that really basic operations can be done.maybe comparing contract instances or something?
reedsa
force-pushed
the
contract-abi-required
branch
from
06255f7
to
4354cd1
Compare
What was wrong?
Lacking clarity on whether abi must be provided.
Closes #2539
How was it fixed?
Explicit docs for ABI being required for contract interactions to work.
Todo:
Cute Animal Picture