Clarify parent_entity requirements for observations_dataframe (#242)

Author: clincoln8

datacommons_client/client.py

@@ -136,17 +136,18 @@ def observations_dataframe(
         entity_type (Optional[str]): The type of entities to filter by when `entity_dcids="all"`.
             Required if `entity_dcids="all"`. Defaults to None.
         parent_entity (Optional[str]): The parent entity under which the target entities fall.
-            Used only when `entity_dcids="all"`. Defaults to None.
+            Required if `entity_dcids="all"`. Defaults to None.
         property_filters (Optional[dict[str, str | list[str]]): An optional dictionary used to filter
             the data by using observation properties like `measurementMethod`, `unit`, or `observationPeriod`.
 
         Returns:
             pd.DataFrame: A DataFrame containing the requested observations.
         """
 
-    if entity_dcids == "all" and not entity_type:
+    if entity_dcids == "all" and not (entity_type and parent_entity):
       raise ValueError(
-          "When 'entity_dcids' is 'all', 'entity_type' must be specified.")
+          "When 'entity_dcids' is 'all', both 'parent_entity' and 'entity_type' must be specified."
+      )
 
     if entity_dcids != "all" and (entity_type or parent_entity):
       raise ValueError(

datacommons_client/tests/test_client.py

@@ -70,11 +70,27 @@ def test_observations_dataframe_raises_error_when_entities_all_but_no_entity_typ
   """Tests that ValueError is raised if 'entities' is 'all' but 'entity_type' is not specified."""
   with pytest.raises(
       ValueError,
-      match="When 'entity_dcids' is 'all', 'entity_type' must be specified.",
+      match=
+      "When 'entity_dcids' is 'all', both 'parent_entity' and 'entity_type' must be specified.",
   ):
     mock_client.observations_dataframe(variable_dcids="var1",
                                        date="2024",
-                                       entity_dcids="all")
+                                       entity_dcids="all",
+                                       parent_entity="africa")
+
+
+def test_observations_dataframe_raises_error_when_entities_all_but_no_parent_entity(
+    mock_client,):
+  """Tests that ValueError is raised if 'entities' is 'all' but 'entity_type' is not specified."""
+  with pytest.raises(
+      ValueError,
+      match=
+      "When 'entity_dcids' is 'all', both 'parent_entity' and 'entity_type' must be specified.",
+  ):
+    mock_client.observations_dataframe(variable_dcids="var1",
+                                       date="2024",
+                                       entity_dcids="all",
+                                       entity_type="Country")
 
 
 def test_observations_dataframe_raises_error_when_invalid_entity_type_usage(