ethereum / py-geth

Python wrapping for running Go-Ethereum as a subprocess
MIT License
100 stars 218 forks source link

Type wrapper #205

Closed pacrob closed 5 months ago

pacrob commented 5 months ago

What was wrong?

Adding types to wrapper.py.

Rather than keeping 2 lists of accepted geth_kwargs (in the args to construct_popen_command and in the GethKwargs model), construct_popen_command now takes the normal **geth_kwargs as its arg, then uses GethKwargs to validate and fill the default values.

Branched from #204, merge that first

How was it fixed?

Todo:

Cute Animal Picture

image

pacrob commented 5 months ago

I couldn't 100% tell the rhyme or reason to delete some options but not others, so feel free to take or leave my comments 😆 typing looks good though. I think you're moving that **geth_kwargs: Any to something stricter?

All these deletions are fields that copilot filled in when I was very first creating the GethKwargs model (should have noted this initially). In this PR, I went though all the kwargs that are actually used anywhere in py-geth and removed the ones that aren't, as there isn't a mechanism for applying additional fields directly (the user can add them via suffix_args and suffix_kwargs if they want to.)

We looked at using a TypedDict to validate the geth_kwargs dict, but to do it properly would break some existing function signatures. Once all the broader typing & cleanup is done, I'll bring proposed breaking changes and see how everyone feels.