Skip to content

Commit 2a95540

Browse files
authored
Improve url handling (#216)
@kmoscoe identified an improvement we can make to how the `DataCommonsClient` class handles a fully resolved URL. For context, during the design phase, @dwnoble requested having a way to provide a fully resolved URL, instead of the URL of Google's Data Commons (datacommons.org) or the URL of a custom Data Commons instance (like datacommons.one.org). That means that the `DataCommonsClient` class is initiated with: ```python """ Args: api_key (Optional[str]): The API key for authentication. Defaults to None. Note that custom DC instances do not currently require an API key. dc_instance (Optional[str]): The Data Commons instance to use. Defaults to "datacommons.org". url (Optional[str]): A custom, fully resolved URL for the Data Commons API. Defaults to None. """ ``` However, as @kmoscoe pointed out, if a `url` is used, it will return an error unless `dc_instance` is set to `None`. That's because both `dc_instance` and `url` cannot be set at the same time... and currently the default value of `dc_instance` is, for convenience, `datacommons.org`. This PR introduces a small check (and it's associated test), to ignore `dc_instance` (if its default value is used) in cases when a `url` is provided. That would allow for something like this to work: ```python from datacommons_client.client import DataCommonsClient client = DataCommonsClient(url="http://localhost:8080/core/api/v2/") ``` Instead of raising an error and requiring to also set `dc_instance` to `None`.
1 parent e7dd8e3 commit 2a95540

File tree

2 files changed

+33
-10
lines changed

2 files changed

+33
-10
lines changed

datacommons_client/client.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ def __init__(
4545
dc_instance (Optional[str]): The Data Commons instance to use. Defaults to "datacommons.org".
4646
url (Optional[str]): A custom, fully resolved URL for the Data Commons API. Defaults to None.
4747
"""
48+
# If a fully resolved URL is provided, and the default dc_instance is used,
49+
# ignore that default value
50+
if dc_instance == "datacommons.org" and url:
51+
dc_instance = None
52+
4853
# Create an instance of the API class which will be injected to the endpoints
4954
self.api = API(api_key=api_key, dc_instance=dc_instance, url=url)
5055

datacommons_client/tests/test_client.py

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ def test_observations_dataframe_raises_error_when_entities_all_but_no_entity_typ
6969
"""Tests that ValueError is raised if 'entities' is 'all' but 'entity_type' is not specified."""
7070
with pytest.raises(
7171
ValueError,
72-
match="When 'entity_dcids' is 'all', 'entity_type' must be specified."):
72+
match="When 'entity_dcids' is 'all', 'entity_type' must be specified.",
73+
):
7374
mock_client.observations_dataframe(variable_dcids="var1",
7475
date="2024",
7576
entity_dcids="all")
@@ -83,10 +84,12 @@ def test_observations_dataframe_raises_error_when_invalid_entity_type_usage(
8384
match="Specify 'entity_type' and 'parent_entity'"
8485
" only when 'entity_dcids' is 'all'.",
8586
):
86-
mock_client.observations_dataframe(variable_dcids="var1",
87-
date="2024",
88-
entity_dcids=["entity1"],
89-
entity_type="Country")
87+
mock_client.observations_dataframe(
88+
variable_dcids="var1",
89+
date="2024",
90+
entity_dcids=["entity1"],
91+
entity_type="Country",
92+
)
9093

9194

9295
def test_observations_dataframe_calls_fetch_observations_by_entity_type(
@@ -95,11 +98,13 @@ def test_observations_dataframe_calls_fetch_observations_by_entity_type(
9598
mock_client.observation.fetch_observations_by_entity_type.return_value.get_observations_as_records.return_value = (
9699
[])
97100

98-
df = mock_client.observations_dataframe(variable_dcids=["var1", "var2"],
99-
date="2024",
100-
entity_dcids="all",
101-
entity_type="Country",
102-
parent_entity="Earth")
101+
df = mock_client.observations_dataframe(
102+
variable_dcids=["var1", "var2"],
103+
date="2024",
104+
entity_dcids="all",
105+
entity_type="Country",
106+
parent_entity="Earth",
107+
)
103108

104109
mock_client.observation.fetch_observations_by_entity_type.assert_called_once_with(
105110
variable_dcids=["var1", "var2"],
@@ -162,3 +167,16 @@ def test_observations_dataframe_returns_dataframe_with_expected_columns(
162167
assert df.iloc[1]["variable"] == "var2"
163168
assert df.iloc[1]["value"] == 200
164169
assert df.iloc[1]["unit"] == "unit2"
170+
171+
172+
@patch(
173+
"datacommons_client.endpoints.base.check_instance_is_valid",
174+
return_value="https://test.url",
175+
)
176+
def test_dc_instance_is_ignored_when_url_is_provided(mock_check_instance):
177+
"""Tests that dc_instance is ignored when a fully resolved URL is provided."""
178+
179+
client = DataCommonsClient(api_key="test_key", url="https://test.url")
180+
181+
# Check that the API base_url is set to the fully resolved url
182+
assert client.api.base_url == "https://test.url"

0 commit comments

Comments
 (0)