Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Refactor Python wrapper templates to use nested methods like TypeScript does #1893

Closed
feuGeneA opened this issue Jun 25, 2019 · 3 comments · Fixed by #1996
Closed

Refactor Python wrapper templates to use nested methods like TypeScript does #1893

feuGeneA opened this issue Jun 25, 2019 · 3 comments · Fixed by #1996

Comments

@feuGeneA
Copy link
Contributor

Current Behavior

Generated wrapper methods distinguish a call from a sendTransaction via a parameter to the contract wrapper method, eg erc20Wrapper.transferFrom(..., send_tx=True).

TypeScript wrappers have a different convention, more like erc20Wrapper.transferFrom.sendTransaction(...).

Expected Behavior

Python wrappers should follow the same convention as TypeScript.

Possible Solution

Context

This will be important in order to add other functionality besides just call vs. sendTransaction, from example estimateGas.

@feuGeneA
Copy link
Contributor Author

@fabioberger Here's my latest thinking on this. Note the (doctested! 😄) usage examples in the class docstring, showing the notation in action. As a demo it's far from polished but it does demonstrate the idea of using instances of named classes to provide the desired notation. And Python permits nested class definitions, so I took advantage of that.

Multi-line lambdas are simply not possible in Python, so declaring object literals with inline method definitions is not really possible. I explored several different obscure ways to try to model this, and this is the only one that really made any sense to me. It looks weird, but the weirdness isn't overly clever, and the weirdness is well encapsulated, and overall I think it's the lesser of many evils. We'll see what I think at the end of the weekend...

"""Demonstrate use of nested classes for dot-notation nested objects."""

class Exchange:  # pylint: disable=too-few-public-methods
    """Wrapper for the 0x Exchange contract.

    >>> exchange = Exchange('Web3.py')

    >>> exchange.fill_order.call('{makerAddress: ...}')
    Trace: inside FillOrderMethod.call({makerAddress: ...}). provider=Web3.py
    'fill results'

    >>> exchange.fill_order.send_transaction('{makerAddress: ...}')
    Trace: inside FillOrderMethod.send_transaction({makerAddress: ...}). provider=Web3.py
    'transaction_hash'
    """

    class FillOrderMethod:
        """A class interface to the variety of ways to call fillOrder."""

        def __init__(self, abi: str, provider: str, contract_address: str):
            """Persist instance data."""
            self.abi = abi
            self.provider = provider
            self.contract_address = contract_address

        def call(self, order: str):
            """Call fillOrder & return its return value but don't transact."""
            print(f"Trace: inside FillOrderMethod.call({order}). provider={self.provider}")
            return 'fill results'

        def send_transaction(self, order: str):
            """What goes here?"""
            print(
                f"Trace: inside FillOrderMethod.send_transaction({order}). "
                + f"provider={self.provider}"
            )
            return 'transaction_hash'

    fill_order: FillOrderMethod

    def __init__(self, provider: str):
        self.provider = provider

        self.fill_order = self.FillOrderMethod(
            "{Exchange.fillOrder ABI}", self.provider, "0xContractAddress"
        )

@feuGeneA
Copy link
Contributor Author

@PirosB3 can you weigh in on my design in the comment above? Is there any other practical way to contrive this "nested method" notation? I considered things like dynamic attributes and namedtuples, but none of it seemed to play nice with type checking...

@PirosB3
Copy link
Contributor

PirosB3 commented Jul 22, 2019

Hi @feuGeneA it actually looks pretty nice. While I've seen several patterns in Django's source code that are similar, I'm confused as to why you would still want to nest the two classes together. Is there any way you could un-nest them instead? (could Exchange and FillOrderMethod be on the same level)?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.