97 incompatible search filters with most recent beliefs only and most…

… recent events only (#147) * refactor: move check for empty source filter Signed-off-by: F.N. Claessen <[email protected]> * fix: apply belief timing filters to subquery, too, before taking the minimum horizon Signed-off-by: F.N. Claessen <[email protected]> * fix: pin pandas Signed-off-by: F.N. Claessen <[email protected]> * docs: mention TB Issue Signed-off-by: F.N. Claessen <[email protected]> --------- Signed-off-by: F.N. Claessen <[email protected]>
SeitaBV · Oct 13, 2023 · 6f3ed82 · 6f3ed82
1 parent 73e6131
commit 6f3ed82
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 38 deletions.
diff --git a/pyproject.toml b/pyproject.toml
@@ -48,7 +48,8 @@ dependencies = [
     # https://pandas.pydata.org/pandas-docs/stable/whatsnew/v1.2.0.html#increased-minimum-version-for-python
     "pandas >= 1.1.5, < 1.2; python_version <= '3.6'",
     "pandas >= 1.1.5, < 1.3; python_version == '3.7'",
-    "pandas >= 1.4.0; python_version > '3.7'",
+    # https://github.com/SeitaBV/timely-beliefs/issues/148
+    "pandas >= 1.4.0, < 2.1; python_version > '3.7'",
     # scipy's setup requires minimal Python versions
     "scipy<1.6; python_version <= '3.6'",
     "scipy<1.8; python_version <= '3.7'",

diff --git a/timely_beliefs/beliefs/classes.py b/timely_beliefs/beliefs/classes.py
@@ -460,6 +460,10 @@ def search_session(  # noqa: C901  # todo: remove after removing deprecated argu
             if sensor is None:
                 raise ValueError("No such sensor")
 
+        # Fast-track empty list of sources
+        if source == []:
+            return BeliefsDataFrame(sensor=sensor, beliefs=[])
+
         # Get bounds on the knowledge horizon (so we can already roughly filter by belief time)
         (
             knowledge_horizon_min,
@@ -471,7 +475,48 @@ def search_session(  # noqa: C901  # todo: remove after removing deprecated argu
             get_bounds=True,
         )
 
-        # Query based on start_time_window
+        def apply_belief_timing_filters(q):
+            """Apply filters that concern the belief timing.
+
+            This includes any custom filters
+            """
+
+            # Apply rough belief time filter
+            if not pd.isnull(
+                beliefs_after
+            ) and belief_utils.extreme_timedeltas_not_equal(
+                knowledge_horizon_min, timedelta.min
+            ):
+                q = q.filter(
+                    cls.event_start
+                    >= beliefs_after + cls.belief_horizon + knowledge_horizon_min
+                )
+            if not pd.isnull(
+                beliefs_before
+            ) and belief_utils.extreme_timedeltas_not_equal(
+                knowledge_horizon_max, timedelta.max
+            ):
+                q = q.filter(
+                    cls.event_start
+                    <= beliefs_before + cls.belief_horizon + knowledge_horizon_max
+                )
+
+            # Apply belief horizon filter
+            if not pd.isnull(horizons_at_least):
+                q = q.filter(cls.belief_horizon >= horizons_at_least)
+            if not pd.isnull(horizons_at_most):
+                q = q.filter(cls.belief_horizon <= horizons_at_most)
+
+            # Apply custom filter criteria and join targets
+            if custom_filter_criteria is not None:
+                q = q.filter(*custom_filter_criteria)
+            if custom_join_targets is not None:
+                for target in custom_join_targets:
+                    q = q.join(target)
+
+            return q
+
+        # Main query
         q = session.query(cls).filter(cls.sensor_id == sensor.id)
 
         # Apply event time filter
@@ -496,32 +541,10 @@ def search_session(  # noqa: C901  # todo: remove after removing deprecated argu
         if not pd.isnull(event_ends_before):
             q = q.filter(cls.event_start + sensor.event_resolution <= event_ends_before)
 
-        # Apply rough belief time filter
-        if not pd.isnull(beliefs_after) and belief_utils.extreme_timedeltas_not_equal(
-            knowledge_horizon_min, timedelta.min
-        ):
-            q = q.filter(
-                cls.event_start
-                >= beliefs_after + cls.belief_horizon + knowledge_horizon_min
-            )
-        if not pd.isnull(beliefs_before) and belief_utils.extreme_timedeltas_not_equal(
-            knowledge_horizon_max, timedelta.max
-        ):
-            q = q.filter(
-                cls.event_start
-                <= beliefs_before + cls.belief_horizon + knowledge_horizon_max
-            )
-
-        # Apply belief horizon filter
-        if not pd.isnull(horizons_at_least):
-            q = q.filter(cls.belief_horizon >= horizons_at_least)
-        if not pd.isnull(horizons_at_most):
-            q = q.filter(cls.belief_horizon <= horizons_at_most)
+        q = apply_belief_timing_filters(q)
 
         # Apply source filter
-        if source == []:
-            return BeliefsDataFrame(sensor=sensor, beliefs=[])
-        elif source is not None:
+        if source is not None:
             sources: list = [source] if not isinstance(source, list) else source
             source_cls = sources[0].__class__
             q = q.join(source_cls).filter(cls.source_id.in_([s.id for s in sources]))
@@ -534,12 +557,14 @@ def search_session(  # noqa: C901  # todo: remove after removing deprecated argu
             most_recent_beliefs_only
             and not most_recent_beliefs_only_incompatible_criteria
         ):
+            subq = session.query(
+                cls.event_start,
+                cls.source_id,
+                func.min(cls.belief_horizon).label("most_recent_belief_horizon"),
+            )
+            # Apply belief timing filters to the subquery, too, before taking the minimum horizon
             subq = (
-                session.query(
-                    cls.event_start,
-                    cls.source_id,
-                    func.min(cls.belief_horizon).label("most_recent_belief_horizon"),
-                )
+                apply_belief_timing_filters(subq)
                 .filter(cls.sensor_id == sensor.id)
                 .group_by(cls.event_start, cls.source_id)
                 .subquery()
@@ -573,13 +598,6 @@ def search_session(  # noqa: C901  # todo: remove after removing deprecated argu
                 ),
             )
 
-        # Apply custom filter criteria and join targets
-        if custom_filter_criteria is not None:
-            q = q.filter(*custom_filter_criteria)
-        if custom_join_targets is not None:
-            for target in custom_join_targets:
-                q = q.join(target)
-
         # Build our DataFrame of beliefs
         df = BeliefsDataFrame(sensor=sensor, beliefs=q.all())