Skip to content

Commit d1ec1e0

Browse files
Merge pull request #736 from Kanaries/codex/fix-data-parser-cache-leak-attributed
[codex] Fix data parser cache leaks with attribution
2 parents 2b2c520 + 13f8098 commit d1ec1e0

5 files changed

Lines changed: 39 additions & 20 deletions

File tree

pygwalker/data_parsers/base.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from typing import Generic, Dict, List, Any, Optional
22
from typing_extensions import Literal
3-
from functools import lru_cache
3+
from functools import cached_property, lru_cache
44
from datetime import datetime, date
55
from datetime import timedelta
66
import abc
@@ -142,16 +142,14 @@ def __init__(
142142
self.infer_number_to_dimension = infer_number_to_dimension
143143
self.other_params = other_params
144144

145-
@property
146-
@lru_cache()
145+
@cached_property
147146
def field_metas(self) -> List[Dict[str, str]]:
148147
duckdb.register("pygwalker_mid_table", self._duckdb_df)
149148
result = duckdb.query("SELECT * FROM pygwalker_mid_table LIMIT 1")
150149
data = result.fetchone()
151150
return get_data_meta_type(dict(zip(result.columns, data))) if data else []
152151

153-
@property
154-
@lru_cache()
152+
@cached_property
155153
def raw_fields(self) -> List[Dict[str, str]]:
156154
return [self._infer_prop(col, self.field_specs) for _, col in enumerate(self._example_df.columns)]
157155

pygwalker/data_parsers/cloud_dataset_parser.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from typing import Any, Dict, List, Optional
2-
from functools import lru_cache
2+
from functools import cached_property
33
from decimal import Decimal
44
import logging
55
import io
@@ -41,14 +41,12 @@ def _get_example_pandas_df(self) -> pd.DataFrame:
4141
example_df[column] = example_df[column].astype(float)
4242
return example_df
4343

44-
@property
45-
@lru_cache()
44+
@cached_property
4645
def field_metas(self) -> List[Dict[str, str]]:
4746
data = self._get_all_datas(1)
4847
return get_data_meta_type(data[0]) if data else []
4948

50-
@property
51-
@lru_cache()
49+
@cached_property
5250
def raw_fields(self) -> List[Dict[str, str]]:
5351
pandas_parser = PandasDataFrameDataParser(
5452
self.example_pandas_df,

pygwalker/data_parsers/database_parser.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from typing import Any, Dict, List, Optional
2-
from functools import lru_cache
2+
from functools import cached_property
33
from decimal import Decimal
44
import logging
55
import json
@@ -179,14 +179,12 @@ def _format_sql(self, sql: str) -> str:
179179
def placeholder_table_name(self) -> str:
180180
return "___pygwalker_temp_view_name___"
181181

182-
@property
183-
@lru_cache()
182+
@cached_property
184183
def field_metas(self) -> List[Dict[str, str]]:
185184
data = self._get_datas_by_sql(f"SELECT * FROM {self.placeholder_table_name} LIMIT 1")
186185
return get_data_meta_type(data[0]) if data else []
187186

188-
@property
189-
@lru_cache()
187+
@cached_property
190188
def raw_fields(self) -> List[Dict[str, str]]:
191189
pandas_parser = PandasDataFrameDataParser(
192190
self.example_pandas_df,

pygwalker/data_parsers/spark_parser.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from typing import Any, Dict, List, Optional
2-
from functools import lru_cache
2+
from functools import cached_property
33
import logging
44
import io
55

@@ -41,8 +41,7 @@ def __init__(
4141
self.infer_number_to_dimension = infer_number_to_dimension
4242
self.other_params = other_params
4343

44-
@property
45-
@lru_cache()
44+
@cached_property
4645
def raw_fields(self) -> List[Dict[str, str]]:
4746
pandas_parser = PandasDataFrameDataParser(
4847
self.example_pandas_df,
@@ -53,8 +52,7 @@ def raw_fields(self) -> List[Dict[str, str]]:
5352
)
5453
return pandas_parser.raw_fields
5554

56-
@property
57-
@lru_cache()
55+
@cached_property
5856
def field_metas(self) -> List[Dict[str, str]]:
5957
data = self.get_datas_by_sql("SELECT * FROM pygwalker_mid_table LIMIT 1")
6058
return get_data_meta_type(data[0]) if data else []

tests/test_data_parsers.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import gc
12
import os.path
3+
import pickle
24
import subprocess
35
import sys
6+
import weakref
47

58
from sqlalchemy import create_engine
69
import pandas as pd
@@ -11,6 +14,7 @@
1114
from pygwalker.services.data_parsers import get_dataset_hash, get_parser
1215
from pygwalker.data_parsers.database_parser import Connector, DatabaseDataParser, text
1316
from pygwalker.data_parsers.database_parser import _check_view_sql
17+
from pygwalker.data_parsers.pandas_parser import PandasDataFrameDataParser
1418
from pygwalker.errors import ViewSqlSameColumnError
1519

1620
datas = [
@@ -94,6 +98,29 @@ def test_get_parser_reports_supported_inputs_for_unsupported_dataset():
9498
assert "cloud dataset id string" in message
9599

96100

101+
@pytest.mark.parametrize("cached_property_name", ["raw_fields", "field_metas"])
102+
def test_pandas_parser_cached_properties_do_not_retain_parser_or_dataframe(cached_property_name):
103+
def create_refs():
104+
df = pd.DataFrame({"city": ["London", "Tokyo"], "value": [1, 2]})
105+
parser = PandasDataFrameDataParser(df, [], True, True, {})
106+
getattr(parser, cached_property_name)
107+
return weakref.ref(parser), weakref.ref(df)
108+
109+
parser_ref, df_ref = create_refs()
110+
gc.collect()
111+
112+
assert parser_ref() is None
113+
assert df_ref() is None
114+
115+
116+
def test_pandas_parser_remains_picklable_after_cached_properties_are_loaded():
117+
parser = PandasDataFrameDataParser(pd.DataFrame({"city": ["London"], "value": [1]}), [], True, True, {})
118+
parser.raw_fields
119+
parser.field_metas
120+
121+
pickle.dumps(parser)
122+
123+
97124
@pytest.mark.parametrize(
98125
"module_name",
99126
[

0 commit comments

Comments
 (0)