Skip to content

fix: fix mypy errors #15482

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.

Sign up for GitHub

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

Closed
parthea wants to merge 1 commit into from
Closed

fix: fix mypy errors #15482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions packages/google-cloud-automl/google/cloud/automl_v1beta1/services/tables/gcs_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@

try:
import pandas
_PANDAS_INSTALLED = True
except ImportError: # pragma: NO COVER
pandas = None
_PANDAS_INSTALLED = False

try:
from google.cloud import storage
# TODO(https://github.com/googleapis/python-storage/issues/318):
# Remove `type: ignore` once this bug is fixed
from google.cloud import storage # type: ignore
_STORAGE_INSTALLED = True
except ImportError: # pragma: NO COVER
storage = None
_STORAGE_INSTALLED = False
Comment on lines 24 to +36
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 23, 2026

Choose a reason for hiding this comment

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

high

This approach of using boolean flags to track optional dependencies can introduce a NameError for static type checkers like mypy. If an import fails, the module name is not defined, which will cause a mypy error on later usage (e.g., pandas.DataFrame).

The original pattern of setting the module to None on ImportError is more robust. The mypy errors can be fixed by other means. For storage, the type: ignore you've added is correct. For pandas, mypy should correctly handle the if pandas is None: raise type guard, especially with pandas-stubs installed.

I suggest reverting this block to the conventional pattern, which will also require reverting the checks below.

Suggested change
try:
import pandas
_PANDAS_INSTALLED = True
except ImportError: # pragma: NO COVER
pandas = None
_PANDAS_INSTALLED = False
try:
from google.cloud import storage
# TODO(https://github.com/googleapis/python-storage/issues/318):
# Remove `type: ignore` once this bug is fixed
from google.cloud import storage # type: ignore
_STORAGE_INSTALLED = True
except ImportError: # pragma: NO COVER
storage = None
_STORAGE_INSTALLED = False
try:
import pandas
except ImportError: # pragma: NO COVER
pandas = None
try:
# TODO(https://github.com/googleapis/python-storage/issues/318):
# Remove `type: ignore` once this bug is fixed
from google.cloud import storage # type: ignore
except ImportError: # pragma: NO COVER
storage = None


_LOGGER = logging.getLogger(__name__)
_PANDAS_REQUIRED = "pandas is required to verify type DataFrame."
Expand Down Expand Up @@ -59,7 +63,7 @@ def __init__(self, bucket_name=None, client=None, credentials=None, project=None
the client will attempt to ascertain the credentials from the
environment.
"""
if storage is None:
if not _STORAGE_INSTALLED:
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 23, 2026

Choose a reason for hiding this comment

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

high

To align with the suggested change to the import pattern, this check should be reverted to if storage is None:.

Suggested change
if not _STORAGE_INSTALLED:
if storage is None:

raise ImportError(_STORAGE_REQUIRED)

if client is not None:
Expand Down Expand Up @@ -119,7 +123,7 @@ def upload_pandas_dataframe(self, dataframe, uploaded_csv_name=None):
Returns:
A string representing the GCS URI of the uploaded CSV.
"""
if pandas is None:
if not _PANDAS_INSTALLED:
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 23, 2026

Choose a reason for hiding this comment

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

high

To align with the suggested change to the import pattern, this check should be reverted to if pandas is None:.

Suggested change
if not _PANDAS_INSTALLED:
if pandas is None:

raise ImportError(_PANDAS_REQUIRED)

if not isinstance(dataframe, pandas.DataFrame):
Expand Down
1 change: 1 addition & 0 deletions packages/google-cloud-automl/noxfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ def mypy(session):
"mypy<1.16.0",
"types-requests",
"types-protobuf",
"pandas-stubs",
)
session.install(".")
session.run(
Expand Down
Loading