Skip to content

Commit e83a661

Browse files
committed
fix: Complete mutable default parameters fix and resolve mypy issues
- Fixed mutable default parameters in all client.py methods - Added proper None handling throughout the codebase - Ensured type safety for mypy by adding assertions and proper handling - All feature flag methods now use None defaults and handle them correctly - Resolved all new mypy violations introduced by the changes This comprehensive fix prevents potential bugs from shared mutable defaults across all feature flag functionality in the PostHog Python client.
1 parent 8737485 commit e83a661

File tree

2 files changed

+50
-32
lines changed

2 files changed

+50
-32
lines changed

posthog/__init__.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
set_context_session as inner_set_context_session,
1212
identify_context as inner_identify_context,
1313
)
14-
from posthog.types import FeatureFlag, FlagsAndPayloads
14+
from posthog.types import FeatureFlag, FlagsAndPayloads, FeatureFlagResult
1515
from posthog.version import VERSION
1616

1717
__version__ = VERSION
@@ -361,7 +361,8 @@ def get_feature_flag_result(
361361
only_evaluate_locally=False,
362362
send_feature_flag_events=True,
363363
disable_geoip=None, # type: Optional[bool]
364-
) -> Optional[FeatureFlagResult]: # type hint for feature flag result
364+
):
365+
# type: (...) -> Optional[FeatureFlagResult]
365366
"""
366367
Get a FeatureFlagResult object which contains the flag result and payload.
367368

posthog/client.py

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ def get_identity_state(passed) -> tuple[str, bool]:
8383

8484

8585
def add_context_tags(properties):
86+
properties = properties or {}
8687
current_context = _get_current_context()
8788
if current_context:
8889
context_tags = current_context.collect_tags()
@@ -327,14 +328,17 @@ def get_feature_flags_and_payloads(
327328
def get_flags_decision(
328329
self,
329330
distinct_id: Optional[ID_TYPES] = None,
330-
groups: Optional[dict] = {},
331+
groups: Optional[dict] = None,
331332
person_properties=None,
332333
group_properties=None,
333334
disable_geoip=None,
334335
) -> FlagsResponse:
335336
"""
336337
Get feature flags decision, using either flags() or decide() API based on rollout.
337338
"""
339+
groups = groups or {}
340+
person_properties = person_properties or {}
341+
group_properties = group_properties or {}
338342

339343
if distinct_id is None:
340344
distinct_id = get_context_distinct_id()
@@ -376,6 +380,7 @@ def capture(
376380
properties = {**(properties or {}), **system_context()}
377381

378382
properties = add_context_tags(properties)
383+
assert properties is not None # Type hint for mypy
379384

380385
(distinct_id, personless) = get_identity_state(distinct_id)
381386

@@ -391,7 +396,7 @@ def capture(
391396
}
392397

393398
if groups:
394-
msg["properties"]["$groups"] = groups
399+
properties["$groups"] = groups
395400

396401
extra_properties: dict[str, Any] = {}
397402
feature_variants: Optional[dict[str, Union[bool, str]]] = {}
@@ -426,7 +431,8 @@ def capture(
426431
extra_properties["$active_feature_flags"] = active_feature_flags
427432

428433
if extra_properties:
429-
msg["properties"] = {**extra_properties, **msg["properties"]}
434+
properties = {**extra_properties, **properties}
435+
msg["properties"] = properties
430436

431437
return self._enqueue(msg, disable_geoip)
432438

@@ -819,11 +825,15 @@ def _compute_flag_locally(
819825
feature_flag,
820826
distinct_id,
821827
*,
822-
groups={},
823-
person_properties={},
824-
group_properties={},
828+
groups=None,
829+
person_properties=None,
830+
group_properties=None,
825831
warn_on_unknown_groups=True,
826832
) -> FlagValue:
833+
groups = groups or {}
834+
person_properties = person_properties or {}
835+
group_properties = group_properties or {}
836+
827837
if feature_flag.get("ensure_experience_continuity", False):
828838
raise InconclusiveMatchError("Flag has experience continuity enabled")
829839

@@ -869,9 +879,9 @@ def feature_enabled(
869879
key,
870880
distinct_id,
871881
*,
872-
groups={},
873-
person_properties={},
874-
group_properties={},
882+
groups=None,
883+
person_properties=None,
884+
group_properties=None,
875885
only_evaluate_locally=False,
876886
send_feature_flag_events=True,
877887
disable_geoip=None,
@@ -897,9 +907,9 @@ def _get_feature_flag_result(
897907
distinct_id: ID_TYPES,
898908
*,
899909
override_match_value: Optional[FlagValue] = None,
900-
groups: Dict[str, str] = {},
901-
person_properties={},
902-
group_properties={},
910+
groups: Optional[Dict[str, str]] = None,
911+
person_properties=None,
912+
group_properties=None,
903913
only_evaluate_locally=False,
904914
send_feature_flag_events=True,
905915
disable_geoip=None,
@@ -909,9 +919,13 @@ def _get_feature_flag_result(
909919

910920
person_properties, group_properties = (
911921
self._add_local_person_and_group_properties(
912-
distinct_id, groups, person_properties, group_properties
922+
distinct_id, groups or {}, person_properties or {}, group_properties or {}
913923
)
914924
)
925+
# Ensure non-None values for type checking
926+
groups = groups or {}
927+
person_properties = person_properties or {}
928+
group_properties = group_properties or {}
915929

916930
flag_result = None
917931
flag_details = None
@@ -995,9 +1009,9 @@ def get_feature_flag_result(
9951009
key,
9961010
distinct_id,
9971011
*,
998-
groups={},
999-
person_properties={},
1000-
group_properties={},
1012+
groups=None,
1013+
person_properties=None,
1014+
group_properties=None,
10011015
only_evaluate_locally=False,
10021016
send_feature_flag_events=True,
10031017
disable_geoip=None,
@@ -1024,9 +1038,9 @@ def get_feature_flag(
10241038
key,
10251039
distinct_id,
10261040
*,
1027-
groups={},
1028-
person_properties={},
1029-
group_properties={},
1041+
groups=None,
1042+
person_properties=None,
1043+
group_properties=None,
10301044
only_evaluate_locally=False,
10311045
send_feature_flag_events=True,
10321046
disable_geoip=None,
@@ -1094,9 +1108,9 @@ def get_feature_flag_payload(
10941108
distinct_id,
10951109
*,
10961110
match_value: Optional[FlagValue] = None,
1097-
groups={},
1098-
person_properties={},
1099-
group_properties={},
1111+
groups=None,
1112+
person_properties=None,
1113+
group_properties=None,
11001114
only_evaluate_locally=False,
11011115
send_feature_flag_events=True,
11021116
disable_geoip=None,
@@ -1237,9 +1251,9 @@ def get_all_flags(
12371251
self,
12381252
distinct_id,
12391253
*,
1240-
groups={},
1241-
person_properties={},
1242-
group_properties={},
1254+
groups=None,
1255+
person_properties=None,
1256+
group_properties=None,
12431257
only_evaluate_locally=False,
12441258
disable_geoip=None,
12451259
) -> Optional[dict[str, Union[bool, str]]]:
@@ -1258,9 +1272,9 @@ def get_all_flags_and_payloads(
12581272
self,
12591273
distinct_id,
12601274
*,
1261-
groups={},
1262-
person_properties={},
1263-
group_properties={},
1275+
groups=None,
1276+
person_properties=None,
1277+
group_properties=None,
12641278
only_evaluate_locally=False,
12651279
disable_geoip=None,
12661280
) -> FlagsAndPayloads:
@@ -1302,10 +1316,13 @@ def _get_all_flags_and_payloads_locally(
13021316
distinct_id: ID_TYPES,
13031317
*,
13041318
groups: Dict[str, Union[str, int]],
1305-
person_properties={},
1306-
group_properties={},
1319+
person_properties=None,
1320+
group_properties=None,
13071321
warn_on_unknown_groups=False,
13081322
) -> tuple[FlagsAndPayloads, bool]:
1323+
person_properties = person_properties or {}
1324+
group_properties = group_properties or {}
1325+
13091326
if self.feature_flags is None and self.personal_api_key:
13101327
self.load_feature_flags()
13111328

0 commit comments

Comments
 (0)