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

start typing #161

Open
wants to merge 5 commits into
base: typed-py-geth
Choose a base branch
from
Open

start typing #161

wants to merge 5 commits into from

Conversation

pacrob
Copy link
Contributor

@pacrob pacrob commented Oct 24, 2023

This is the initial experimental typing effort. Pay no attention to this PR, just here for reference. It will be closed, not merged.

What was wrong?

py-geth has no types!

Related to Issue #116

How was it fixed?

Added mypy and typed files.

Converted to using pydantic models for GethKwargs and GenesisData.

Todo:

  • Clean up commit history

  • clean TODOs

  • clean breakpoints

  • Add or update documentation related to these changes

  • Add entry to the release notes

Cute Animal Picture

image

geth/utils/encoding.py Outdated Show resolved Hide resolved
geth/utils/encoding.py Outdated Show resolved Hide resolved
@pacrob pacrob changed the title add mypy to deps and start typing start typing Feb 28, 2024
geth/models.py Outdated Show resolved Hide resolved
geth/process.py Outdated Show resolved Hide resolved
geth/process.py Outdated Show resolved Hide resolved
geth/process.py Outdated
return not self.geth_kwargs.get("ipc_disable", None)

@property
def ipc_path(self):
def ipc_path(self) -> str:
return self.geth_kwargs.get(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here too. I think it might look something like this:

@property
def ipc_path(self) -> str:
    return getattr(self.geth_kwargs, "ipc_path", os.path.abspath(
        os.path.expanduser(
            os.path.join(
                self.geth_kwargs.data_dir,
                "geth.ipc",
            )
        )
    ))

@pacrob pacrob force-pushed the start-typing branch 2 times, most recently from e8dcf11 to 27f4834 Compare March 17, 2024 19:42
return is_same_path(data_dir, get_live_data_dir())


def is_ropsten_chain(data_dir):
def is_ropsten_chain(data_dir: str) -> bool:
return is_same_path(data_dir, get_ropsten_data_dir())


def write_genesis_file(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

write_genesis_file currently contains separate parameters clique_period and clique_epoch. But, the circleci output shows that the function receives a clique from GenesisData, I can suppose, that containing both period and epoch. And I think, we should update signature to accepting a single clique parameter

Like this:

def write_genesis_file(
    ...
    clique: Optional[Dict[str, int]] = None,
):
    ...
    if clique is None:
        clique = {"period": 5, "epoch": 30000}
    if config is None:
        config = {
            ...
            "clique": {"period": clique.get("period"), "epoch": clique.get("epoch")},
        }
    ...

@pacrob pacrob changed the base branch from main to add-typing March 27, 2024 20:01
@pacrob pacrob force-pushed the start-typing branch 5 times, most recently from d2dff2a to a9d946b Compare April 26, 2024 21:45
@pacrob pacrob changed the base branch from add-typing to main April 29, 2024 16:56
@pacrob pacrob closed this Apr 29, 2024
@pacrob pacrob deleted the start-typing branch April 29, 2024 16:57
@pacrob pacrob restored the start-typing branch April 29, 2024 17:14
@pacrob pacrob reopened this Apr 29, 2024
@pacrob pacrob mentioned this pull request Apr 29, 2024
3 tasks
@pacrob pacrob changed the base branch from main to typed-py-geth April 30, 2024 17:26
pacrob and others added 3 commits April 30, 2024 11:31
Co-authored-by: David Dzhalaev <72649244+DavidRomanovizc@users.noreply.github.com>

Co-authored-by: David Dzhalaev <72649244+DavidRomanovizc@users.noreply.github.com>
@pacrob pacrob force-pushed the typed-py-geth branch 2 times, most recently from 7831e9b to 92c80f0 Compare May 13, 2024 21:31
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.

None yet

2 participants