zilto commented on code in PR #1376: URL: https://github.com/apache/hamilton/pull/1376#discussion_r2328745233
########## hamilton/base.py: ########## @@ -20,21 +20,22 @@ It cannot import hamilton.graph, or hamilton.driver. """ +from __future__ import annotations + import abc import collections import logging -from typing import Any, Dict, List, Optional, Tuple, Type, Union - -import numpy as np -import pandas as pd -from pandas.core.indexes import extension as pd_extension +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Type, Union +from hamilton import htypes from hamilton.lifecycle import api as lifecycle_api -try: - from . import htypes, node -except ImportError: - import node +if TYPE_CHECKING: Review Comment: Changes: - I moved the imports `pandas`, `numpy` and `pandas.core.indexes.extension` from the top-level to the code path that actually use these dependencies. There should be no behavior change, but it allows to `import hamilton.base` without loading pandas each time. - [if TYPE_CHECKING](https://docs.python.org/3/library/typing.html#typing.TYPE_CHECKING) is the standard Python approach to import package that are only relevant for annotating signatures. For example, `mypy` will import `pandas` when doing type checking, but doing `from hamilton import base` won't import `pandas` - `pandas` and `numpy` are in the type checking block because they are used in some function signatures. `pandas.core.indexes.extension` is not because it isn't used in type annotations. - moved `hamilton.node` to `TYPE_CHECKING` block since it's only used for annotations - moved `htypes` to top-level import; it should not have been in the a `try/except` in the first place because a code path of `SimplePythonDataFrameGraphAdapter` depends on it and will fail error if `htypes` isn't imported I don't have the "why" for this code: ```python try: from . import htypes, node except ImportError: import node ``` - The `try/except` was [introduced in 2022](https://github.com/apache/hamilton/commit/0dcd52a71c32c3b4b43245c2603efa93b645ae29#diff-a60b4bde51922e7c95719941221e63dc0b991a8495b530c1997967f5294e73eb), but no clear indications why. - Looking at the source code of [the file at the time](https://github.com/apache/hamilton/blob/0dcd52a71c32c3b4b43245c2603efa93b645ae29/hamilton/base.py), it was probably a brute force solution to avoid circular imports. - The code could have been in a `TYPE_CHECKING` block (introduced in Python 3.5) since it was only ever used for annotations -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
