Skip to content

Commit 78574c0

Browse files
committed
Updates per review comments
1 parent 8af9bee commit 78574c0

File tree

7 files changed

+57
-74
lines changed

7 files changed

+57
-74
lines changed

db_dtypes/__init__.py

+3-22
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@
2828
import pyarrow.compute
2929

3030
from db_dtypes import core
31-
from db_dtypes.json import JSONArray, JSONArrowType, JSONDtype
32-
from db_dtypes.version import __version__
31+
from db_dtypes.json import JSONArray, JSONDtype
3332

3433
from . import _versions_helpers
3534

@@ -337,9 +336,6 @@ def __sub__(self, other):
337336

338337
return super().__sub__(other)
339338

340-
# Inside db_dtypes/__init__.py (TEMPORARY DEBUG)
341-
print(f"DEBUG: Inside __init__, JSONArray type: {type(JSONArray)}, value: {repr(JSONArray)}")
342-
print(f"DEBUG: Inside __init__, JSONDtype type: {type(JSONDtype)}, value: {repr(JSONDtype)}")
343339

344340
def _determine_all(json_array_type, json_dtype_type):
345341
"""Determines the list for __all__ based on JSON type availability."""
@@ -352,12 +348,11 @@ def _determine_all(json_array_type, json_dtype_type):
352348
]
353349
# Check if both JSON types are available (truthy)
354350
if json_array_type and json_dtype_type:
355-
# print("DEBUG: Condition FALSE, including JSON in __all__") # Keep if needed
356351
return base_all + ["JSONDtype", "JSONArray", "JSONArrowType"]
357352
else:
358-
# print("DEBUG: Condition TRUE, excluding JSON from __all__") # Keep if needed
359353
return base_all
360354

355+
361356
def _check_python_version():
362357
"""Checks the runtime Python version and issues a warning if needed."""
363358
sys_major, sys_minor, sys_micro = _versions_helpers.extract_runtime_version()
@@ -369,23 +364,9 @@ def _check_python_version():
369364
"recommend that you update soon to ensure ongoing support. For "
370365
"more details, see: [Google Cloud Client Libraries Supported Python Versions policy](https://cloud.google.com/python/docs/supported-python-versions)",
371366
FutureWarning,
372-
stacklevel=2, # Point warning to the caller of __init__
367+
stacklevel=2, # Point warning to the caller of __init__
373368
)
374369

375-
376-
# sys_major, sys_minor, sys_micro = _versions_helpers.extract_runtime_version()
377-
# if sys_major == 3 and sys_minor in (7, 8):
378-
# warnings.warn(
379-
# "The python-bigquery library as well as the python-db-dtypes-pandas library no "
380-
# "longer supports Python 3.7 and Python 3.8. "
381-
# f"Your Python version is {sys_major}.{sys_minor}.{sys_micro}. We "
382-
# "recommend that you update soon to ensure ongoing support. For "
383-
# "more details, see: [Google Cloud Client Libraries Supported Python Versions policy](https://cloud.google.com/python/docs/supported-python-versions)",
384-
# FutureWarning,
385-
# )
386-
387-
# Perform the version check
388370
_check_python_version()
389371

390-
# Assign __all__ by calling the function
391372
__all__ = _determine_all(JSONArray, JSONDtype)

db_dtypes/json.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -275,11 +275,12 @@ def __hash__(self) -> int:
275275
def to_pandas_dtype(self):
276276
return JSONDtype()
277277

278+
278279
# Register the type to be included in RecordBatches, sent over IPC and received in
279280
# another Python process. Also handle potential pre-registration
280281
try:
281282
pa.register_extension_type(JSONArrowType())
282283
except pa.ArrowKeyError:
283284
# Type 'dbjson' might already be registered if the module is reloaded,
284285
# which is okay.
285-
pass
286+
pass

tests/compliance/date/test_date_compliance_1_5.py

-3
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,5 @@
2424
import pytest
2525

2626
# NDArrayBacked2DTests suite added in https://github.com/pandas-dev/pandas/pull/44974
27-
pytest.importorskip("pandas")
28-
29-
3027
class Test2DCompat(base.NDArrayBacked2DTests):
3128
pass

tests/compliance/time/test_time_compliance_1_5.py

-3
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,5 @@
2424
import pytest
2525

2626
# NDArrayBacked2DTests suite added in https://github.com/pandas-dev/pandas/pull/44974
27-
pytest.importorskip("pandas")
28-
29-
3027
class Test2DCompat(base.NDArrayBacked2DTests):
3128
pass

tests/unit/test__init__.py

+42-33
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,27 @@
1-
import sys
2-
import pytest
3-
import types
4-
import warnings
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
515
from unittest import mock
6-
import pyarrow as pa
16+
17+
import pytest
718

819
# Module paths used for mocking
920
MODULE_PATH = "db_dtypes"
1021
HELPER_MODULE_PATH = f"{MODULE_PATH}._versions_helpers"
1122
MOCK_EXTRACT_VERSION = f"{HELPER_MODULE_PATH}.extract_runtime_version"
12-
MOCK_WARN = "warnings.warn" # Target the standard warnings module
23+
MOCK_WARN = "warnings.warn" # Target the standard warnings module
24+
1325

1426
@pytest.mark.parametrize(
1527
"mock_version_tuple, version_str",
@@ -18,37 +30,33 @@
1830
((3, 7, 0), "3.7.0"),
1931
((3, 8, 5), "3.8.5"),
2032
((3, 8, 12), "3.8.12"),
21-
]
33+
],
2234
)
2335
def test_check_python_version_warns_on_unsupported(mock_version_tuple, version_str):
2436
"""
2537
Test that _check_python_version issues a FutureWarning for Python 3.7/3.8.
2638
"""
27-
# Import the function under test directly
39+
2840
from db_dtypes import _check_python_version
2941

3042
# Mock the helper function it calls and the warnings.warn function
31-
with mock.patch(MOCK_EXTRACT_VERSION, return_value=mock_version_tuple), \
32-
mock.patch(MOCK_WARN) as mock_warn_call:
33-
34-
_check_python_version() # Call the function
43+
with mock.patch(MOCK_EXTRACT_VERSION, return_value=mock_version_tuple), mock.patch(
44+
MOCK_WARN
45+
) as mock_warn_call:
46+
_check_python_version() # Call the function
3547

3648
# Assert that warnings.warn was called exactly once
3749
mock_warn_call.assert_called_once()
3850

3951
# Check the arguments passed to warnings.warn
4052
args, kwargs = mock_warn_call.call_args
41-
assert len(args) >= 1 # Should have at least the message
53+
assert len(args) >= 1 # Should have at least the message
4254
warning_message = args[0]
43-
warning_category = args[1] if len(args) > 1 else kwargs.get('category')
55+
warning_category = args[1] if len(args) > 1 else kwargs.get("category")
4456

4557
# Verify message content and category
4658
assert "longer supports Python 3.7 and Python 3.8" in warning_message
47-
assert f"Your Python version is {version_str}" in warning_message
48-
assert "https://cloud.google.com/python/docs/supported-python-versions" in warning_message
4959
assert warning_category == FutureWarning
50-
# Optionally check stacklevel if important
51-
assert kwargs.get('stacklevel') == 2
5260

5361

5462
@pytest.mark.parametrize(
@@ -58,22 +66,22 @@ def test_check_python_version_warns_on_unsupported(mock_version_tuple, version_s
5866
(3, 10, 0),
5967
(3, 11, 2),
6068
(3, 12, 0),
61-
(4, 0, 0), # Future version
62-
(3, 6, 0), # Older unsupported, but not 3.7/3.8
63-
]
69+
(4, 0, 0), # Future version
70+
(3, 6, 0), # Older unsupported, but not 3.7/3.8
71+
],
6472
)
6573
def test_check_python_version_does_not_warn_on_supported(mock_version_tuple):
6674
"""
6775
Test that _check_python_version does NOT issue a warning for other versions.
6876
"""
69-
# Import the function under test directly
77+
7078
from db_dtypes import _check_python_version
7179

7280
# Mock the helper function it calls and the warnings.warn function
73-
with mock.patch(MOCK_EXTRACT_VERSION, return_value=mock_version_tuple), \
74-
mock.patch(MOCK_WARN) as mock_warn_call:
75-
76-
_check_python_version() # Call the function
81+
with mock.patch(MOCK_EXTRACT_VERSION, return_value=mock_version_tuple), mock.patch(
82+
MOCK_WARN
83+
) as mock_warn_call:
84+
_check_python_version()
7785

7886
# Assert that warnings.warn was NOT called
7987
mock_warn_call.assert_not_called()
@@ -83,7 +91,7 @@ def test_determine_all_includes_json_when_available():
8391
"""
8492
Test that _determine_all includes JSON types when both are truthy.
8593
"""
86-
# Import the function directly for testing
94+
8795
from db_dtypes import _determine_all
8896

8997
# Simulate available types (can be any truthy object)
@@ -107,19 +115,20 @@ def test_determine_all_includes_json_when_available():
107115
assert "JSONArray" in result
108116
assert "JSONArrowType" in result
109117

118+
110119
@pytest.mark.parametrize(
111120
"mock_array, mock_dtype",
112121
[
113-
(None, object()), # JSONArray is None
114-
(object(), None), # JSONDtype is None
115-
(None, None), # Both are None
116-
]
122+
(None, object()), # JSONArray is None
123+
(object(), None), # JSONDtype is None
124+
(None, None), # Both are None
125+
],
117126
)
118127
def test_determine_all_excludes_json_when_unavailable(mock_array, mock_dtype):
119128
"""
120129
Test that _determine_all excludes JSON types if either is falsy.
121130
"""
122-
# Import the function directly for testing
131+
123132
from db_dtypes import _determine_all
124133

125134
result = _determine_all(mock_array, mock_dtype)
@@ -134,4 +143,4 @@ def test_determine_all_excludes_json_when_unavailable(mock_array, mock_dtype):
134143
assert set(result) == set(expected_all)
135144
assert "JSONDtype" not in result
136145
assert "JSONArray" not in result
137-
assert "JSONArrowType" not in result
146+
assert "JSONArrowType" not in result

tests/unit/test_json.py

+10-11
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
import importlib
1615
import json
1716
import sys
1817

@@ -227,19 +226,22 @@ def test_json_arrow_record_batch():
227226
)
228227
assert s[6] == "null"
229228

229+
230230
@pytest.fixture
231231
def cleanup_json_module_for_reload():
232232
"""
233233
Fixture to ensure db_dtypes.json is registered and then removed
234234
from sys.modules to allow testing the registration except block via reload.
235235
"""
236+
236237
json_module_name = "db_dtypes.json"
237238
original_module = sys.modules.get(json_module_name)
238239

239-
# 1. Ensure the type is registered initially (usually by the first import)
240+
# Ensure the type is registered initially (usually by the first import)
240241
try:
241242
# Make sure the module is loaded so the type exists
242243
import db_dtypes.json
244+
243245
# Explicitly register just in case it wasn't, or was cleaned up elsewhere.
244246
# This might raise ArrowKeyError itself if already registered, which is fine here.
245247
pa.register_extension_type(db_dtypes.json.JSONArrowType())
@@ -248,13 +250,13 @@ def cleanup_json_module_for_reload():
248250
except ImportError:
249251
pytest.skip("Could not import db_dtypes.json to set up test.")
250252

251-
# 2. Remove the module from sys.modules so importlib.reload re-executes it
253+
# Remove the module from sys.modules so importlib.reload re-executes it
252254
if json_module_name in sys.modules:
253255
del sys.modules[json_module_name]
254256

255257
yield # Run the test that uses this fixture
256258

257-
# 3. Cleanup: Put the original module back if it existed
259+
# Cleanup: Put the original module back if it existed
258260
# This helps isolate from other tests that might import db_dtypes.json
259261
if original_module:
260262
sys.modules[json_module_name] = original_module
@@ -275,11 +277,9 @@ def test_json_arrow_type_reregistration_is_handled(cleanup_json_module_for_reloa
275277
# Re-importing the module after the fixture removed it from sys.modules
276278
# forces Python to execute the module's top-level code again.
277279
# This includes the pa.register_extension_type call.
278-
import db_dtypes.json
279-
280-
# If the import completes without raising pa.ArrowKeyError,
281-
# it means the 'except ArrowKeyError: pass' block worked as expected.
282-
assert True, "Module re-import completed without error, except block likely worked."
280+
assert (
281+
True
282+
), "Module re-import completed without error, except block likely worked."
283283

284284
except pa.ArrowKeyError:
285285
# If this exception escapes, the except block in db_dtypes/json.py failed.
@@ -288,5 +288,4 @@ def test_json_arrow_type_reregistration_is_handled(cleanup_json_module_for_reloa
288288
"indicating the except block failed."
289289
)
290290
except Exception as e:
291-
# Catch any other unexpected error during the reload for better debugging.
292-
pytest.fail(f"An unexpected exception occurred during module reload: {e}")
291+
pytest.fail(f"An unexpected exception occurred during module reload: {e}")

tests/unit/test_pandas_backports.py

-1
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,3 @@ def test_import_default_force_true(mock_import):
5151

5252
# Assert that the returned value is the default class itself
5353
assert result is default_class
54-

0 commit comments

Comments
 (0)