Skip to content

Commit a36d77e

Browse files
committed
fix: address comments
1 parent 074dc5c commit a36d77e

File tree

8 files changed

+51
-130
lines changed

8 files changed

+51
-130
lines changed

.basedpyright/baseline.json

Lines changed: 0 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -9103,38 +9103,6 @@
91039103
"lineCount": 1
91049104
}
91059105
},
9106-
{
9107-
"code": "reportUnknownParameterType",
9108-
"range": {
9109-
"startColumn": 60,
9110-
"endColumn": 66,
9111-
"lineCount": 1
9112-
}
9113-
},
9114-
{
9115-
"code": "reportMissingParameterType",
9116-
"range": {
9117-
"startColumn": 60,
9118-
"endColumn": 66,
9119-
"lineCount": 1
9120-
}
9121-
},
9122-
{
9123-
"code": "reportUnknownArgumentType",
9124-
"range": {
9125-
"startColumn": 36,
9126-
"endColumn": 42,
9127-
"lineCount": 1
9128-
}
9129-
},
9130-
{
9131-
"code": "reportUnknownArgumentType",
9132-
"range": {
9133-
"startColumn": 36,
9134-
"endColumn": 42,
9135-
"lineCount": 1
9136-
}
9137-
},
91389106
{
91399107
"code": "reportImplicitStringConcatenation",
91409108
"range": {
@@ -9151,53 +9119,13 @@
91519119
"lineCount": 1
91529120
}
91539121
},
9154-
{
9155-
"code": "reportUnknownParameterType",
9156-
"range": {
9157-
"startColumn": 60,
9158-
"endColumn": 66,
9159-
"lineCount": 1
9160-
}
9161-
},
9162-
{
9163-
"code": "reportMissingParameterType",
9164-
"range": {
9165-
"startColumn": 60,
9166-
"endColumn": 66,
9167-
"lineCount": 1
9168-
}
9169-
},
91709122
{
91719123
"code": "reportUnusedCallResult",
91729124
"range": {
91739125
"startColumn": 8,
91749126
"endColumn": 43,
91759127
"lineCount": 1
91769128
}
9177-
},
9178-
{
9179-
"code": "reportUnknownArgumentType",
9180-
"range": {
9181-
"startColumn": 36,
9182-
"endColumn": 42,
9183-
"lineCount": 1
9184-
}
9185-
},
9186-
{
9187-
"code": "reportUnknownArgumentType",
9188-
"range": {
9189-
"startColumn": 36,
9190-
"endColumn": 42,
9191-
"lineCount": 1
9192-
}
9193-
},
9194-
{
9195-
"code": "reportUnknownArgumentType",
9196-
"range": {
9197-
"startColumn": 36,
9198-
"endColumn": 42,
9199-
"lineCount": 1
9200-
}
92019129
}
92029130
],
92039131
"./src/metaxy/models/field.py": [

src/metaxy/models/feature_spec.py

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
Protocol,
1414
TypeAlias,
1515
TypeVar,
16-
cast,
1716
overload,
1817
runtime_checkable,
1918
)
@@ -209,26 +208,6 @@ def table_name(self) -> str:
209208
) # bound, should be used for generic
210209

211210

212-
def _coerce_metadata(value: Any) -> dict[str, JsonValue] | None:
213-
if value is None:
214-
return None
215-
if not isinstance(value, Mapping):
216-
raise ValueError("metadata must be a mapping")
217-
try:
218-
serialized = json.dumps(value)
219-
except (TypeError, ValueError) as exc:
220-
raise ValueError(
221-
"metadata must be JSON-serializable. Found non-serializable value"
222-
) from exc
223-
return cast(dict[str, JsonValue], json.loads(serialized))
224-
225-
226-
MetadataField = Annotated[
227-
dict[str, JsonValue] | None,
228-
BeforeValidator(_coerce_metadata),
229-
]
230-
231-
232211
class _BaseFeatureSpec(FrozenBaseModel):
233212
key: Annotated[FeatureKey, BeforeValidator(FeatureKeyAdapter.validate_python)]
234213
deps: list[FeatureDep] | None = None
@@ -241,8 +220,8 @@ class _BaseFeatureSpec(FrozenBaseModel):
241220
)
242221
]
243222
)
244-
metadata: MetadataField = pydantic.Field(
245-
default=None,
223+
metadata: dict[str, JsonValue] = pydantic.Field(
224+
default_factory=dict,
246225
description="Metadata attached to this feature.",
247226
)
248227

@@ -258,7 +237,7 @@ def __init__(
258237
deps: list[FeatureDep] | None = None,
259238
fields: list[FieldSpec] | None = None,
260239
id_columns: list[str] | None = None,
261-
metadata: Mapping[str, Any] | JsonValue | None = None,
240+
metadata: Mapping[str, JsonValue] | None = None,
262241
) -> None:
263242
"""Initialize from string key."""
264243
...
@@ -271,7 +250,7 @@ def __init__(
271250
deps: list[FeatureDep] | None = None,
272251
fields: list[FieldSpec] | None = None,
273252
id_columns: list[str] | None = None,
274-
metadata: Mapping[str, Any] | JsonValue | None = None,
253+
metadata: Mapping[str, JsonValue] | None = None,
275254
) -> None:
276255
"""Initialize from sequence of parts."""
277256
...
@@ -284,7 +263,7 @@ def __init__(
284263
deps: list[FeatureDep] | None = None,
285264
fields: list[FieldSpec] | None = None,
286265
id_columns: list[str] | None = None,
287-
metadata: Mapping[str, Any] | JsonValue | None = None,
266+
metadata: Mapping[str, JsonValue] | None = None,
288267
) -> None:
289268
"""Initialize from FeatureKey instance."""
290269
...
@@ -297,11 +276,12 @@ def __init__(
297276
deps: list[FeatureDep] | None = None,
298277
fields: list[FieldSpec] | None = None,
299278
id_columns: list[str] | None = None,
279+
metadata: Mapping[str, JsonValue] | None = None,
300280
) -> None:
301281
"""Initialize from BaseFeatureSpec instance."""
302282
...
303283

304-
def __init__(self, key: CoercibleToFeatureKey | Self, **kwargs):
284+
def __init__(self, key: CoercibleToFeatureKey | Self, **kwargs: Any):
305285
if isinstance(key, type(self)):
306286
key = key.key
307287
else:
@@ -384,8 +364,6 @@ def feature_spec_version(self) -> str:
384364
# Use model_dump with mode="json" for deterministic serialization
385365
# This ensures all types (like FeatureKey) are properly serialized
386366
spec_dict = self.model_dump(mode="json")
387-
if spec_dict.get("metadata") == {}:
388-
spec_dict.pop("metadata", None)
389367

390368
# Sort keys to ensure deterministic ordering
391369
spec_json = json.dumps(spec_dict, sort_keys=True)
@@ -422,6 +400,7 @@ def __init__(
422400
deps: list[FeatureDep] | None = None,
423401
fields: list[FieldSpec] | None = None,
424402
id_columns: Sequence[str] | None = None,
403+
metadata: Mapping[str, JsonValue] | None = None,
425404
) -> None:
426405
"""Initialize from string key."""
427406
...
@@ -434,6 +413,7 @@ def __init__(
434413
deps: list[FeatureDep] | None = None,
435414
fields: list[FieldSpec] | None = None,
436415
id_columns: Sequence[str] | None = None,
416+
metadata: Mapping[str, JsonValue] | None = None,
437417
) -> None:
438418
"""Initialize from sequence of parts."""
439419
...
@@ -446,6 +426,7 @@ def __init__(
446426
deps: list[FeatureDep] | None = None,
447427
fields: list[FieldSpec] | None = None,
448428
id_columns: Sequence[str] | None = None,
429+
metadata: Mapping[str, JsonValue] | None = None,
449430
) -> None:
450431
"""Initialize from FeatureKey instance."""
451432
...
@@ -458,11 +439,12 @@ def __init__(
458439
deps: list[FeatureDep] | None = None,
459440
fields: list[FieldSpec] | None = None,
460441
id_columns: Sequence[str] | None = None,
442+
metadata: Mapping[str, JsonValue] | None = None,
461443
) -> None:
462444
"""Initialize from FeatureSpec instance."""
463445
...
464446

465-
def __init__(self, key: CoercibleToFeatureKey | Self, **kwargs):
447+
def __init__(self, key: CoercibleToFeatureKey | Self, **kwargs: Any):
466448
# id_columns is always set for FeatureSpec
467449
super().__init__(key=key, **kwargs)
468450

tests/__snapshots__/test_feature_project_detection.ambr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
# name: test_feature_project_persists_across_graph_operations
1212
dict({
1313
'project': 'persist_project',
14-
'tracking_version': '2a72c9e2',
14+
'tracking_version': '47e9595c',
1515
})
1616
# ---
1717
# name: test_multiple_features_same_project

tests/__snapshots__/test_feature_tracking_version.ambr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
'feature_versions_same': True,
1313
'project_a': 'project_a',
1414
'project_b': 'project_b',
15-
'tracking_version_a': '7da02a89202f8eb9f19782512fadf988b815df8255e52a1f35dec9da76e18b12',
16-
'tracking_version_b': '5908b5019bdc1acd103482f1d144338f407f50d9d1aa09ca3808fe376f5ffd98',
15+
'tracking_version_a': '51105d501a6e001dcbb7c3edb9e4ce93b12a26dc1d0b445cca1de9e986edd3a1',
16+
'tracking_version_b': 'e0246e418b8079fd6568fe80959809d0c4161b3c8e4ce70a0e4ecb0adeee0d1b',
1717
'tracking_versions_differ': True,
1818
})
1919
# ---
@@ -43,5 +43,5 @@
4343
})
4444
# ---
4545
# name: test_tracking_version_deterministic
46-
'139c7982886b761076245c878e6ad545f4afb9ffabab9238d7af883aff161b9b'
46+
'93c7499560ffbab47bb760edd62a6c6be19c7d3ac6977b0286ab5eea4716c23a'
4747
# ---
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
# serializer version: 1
22
# name: test_snapshot_stability_with_id_columns
33
dict({
4-
'composite': '8dc629cb2bc4bf404e15495f03c7efdcb339b96531fe6579d3f927a82589f843',
5-
'default': '0f5dac77217aa051fd504566de0fa3e1d8d4a360ed7958e716ff0f6ebb483a34',
6-
'single_custom': 'cfca9196fcb832b9da472042d9fd1ae5c485352d8e409d3d7c1f739880249c22',
4+
'composite': 'd51aec13661dd2d2aea4a832b894c93d73ea5d4f5173202ec03ea2e1f483fc6e',
5+
'default': '2f4f9c1127afb2868dcd21018adcbfd3680f4eab9e9fb9aefe96d6c9f7a1bd24',
6+
'single_custom': 'c571093281edaecb612ae310a509e20047688f00016d1e8fdf0c01361144f244',
77
})
88
# ---

tests/__snapshots__/test_spec_version.ambr

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
# serializer version: 1
22
# name: test_feature_spec_version_deterministic
3-
'b0741037ecd180b5948761b9d73b4dd78efa9cf7224240d3e3b718bae26e5852'
3+
'90179e7bd99745bd2494f695fa18c15937353a3788c9b5c4650a81a071c1f591'
44
# ---
55
# name: test_feature_spec_version_includes_all_properties
6-
'8b54ae3cd476f3071a9437d4626658c15e40005380a0e2093585d442866e167b'
6+
'e7c76b7981649ad1e0d9c8f373043eb42ef75feb7f1a5e222236afb632f12e97'
77
# ---
88
# name: test_feature_spec_version_recorded_in_metadata_store
99
dict({
1010
'feature_key': 'recorded/feature',
11-
'feature_spec_version': '39bcce7c7d9bc23f78982655717dc56879265b87304a96f371c7935df5c59d54',
11+
'feature_spec_version': 'd64c6d4dcd782fda8988b6f789367a2c10aed2732e350eb3e2a285051c73d493',
1212
'feature_version': '0c7b2d83252fbf2f689bec7d37c4f7ffb103755d11eafabc1e000a372f415d83',
1313
'snapshot_version': '14d4294da40f4ab27ca85eb739fffd70e92c92522dbcba995cfa2aa343988bf3',
1414
})
@@ -34,17 +34,18 @@
3434
'snapshot',
3535
'test',
3636
]),
37-
'metadata': None,
37+
'metadata': dict({
38+
}),
3839
}),
39-
'feature_spec_version': 'ceaebb2a96f45752b2ca7daf8326b893d029cc56b366d7ee67bd4a3667008a03',
40-
'feature_tracking_version': '7dac08b916cbef01ec6971f5dd00a1c520c31ed6c09d1fd2223e82ed418546fb',
40+
'feature_spec_version': '6dc07209be338e7e86bb47dc3f376f3e7447efe35510cac9119ee0878c8c20a2',
41+
'feature_tracking_version': 'af52f7692de124d551c0753a169b2a0b4a57003818ed08474c6d9c17480b7e20',
4142
'feature_version': '7cfde77960e3cf10327e9cb97f311418b101fc5ee2c146922a997306e318edaf',
4243
'project': 'test',
4344
})
4445
# ---
4546
# name: test_feature_spec_version_with_column_selection_and_rename
46-
'a412fd83810799e529173eed40fbcc29ed9adaed8c37ac4ffb21edd4f47c60d1'
47+
'cf22f9791d5b83dc084c493e52914a21c6fe27d261e78ec03fe22e033f750b4a'
4748
# ---
4849
# name: test_feature_spec_version_with_multiple_complex_deps
49-
'09d45d5f79ef381519aad8a30dae3c2bb253f8bc2e61c63114b8ac48fa8efcbc'
50+
'20703ec5548c00dbc3f0d634a6791f284d6b1cec4701d014fe5ccaec4deef74b'
5051
# ---

tests/metadata_stores/__snapshots__/test_snapshot_push.ambr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,19 @@
2929
'versions': list([
3030
dict({
3131
'feature_key': 'downstream',
32-
'feature_spec_version': '20d33a795e3990ca63dbde8a5cd9ee3ab48738c4c1f6375cdcbc638202c505e5',
32+
'feature_spec_version': '5596900623b161e09c480056de59559d5c88e8c98718691fe6736988ef716f58',
3333
'feature_version': '7b8e38a11f800fb93c18c0e04c4609d7b52ec12b62177ba8355531d363f01751',
3434
'snapshot_version': '1ab4ecd9d7af535b843dc22416dd99d5e430317c0a2bcd01fe83ffc2d7392da8',
3535
}),
3636
dict({
3737
'feature_key': 'upstream',
38-
'feature_spec_version': 'a8e3517bbf15ca3ecc14d6300c3c78ace14765963993cfc5ffbee8512c6a7655',
38+
'feature_spec_version': '72c4fbce316d0efb1a2f90c2dc45b44a2fa97a09f2397423b51c06b591c289c3',
3939
'feature_version': '8a2ffeab8da447095c5ee7a77e5635a1e16e7f3605021732f50f7002fa258398',
4040
'snapshot_version': '1ab4ecd9d7af535b843dc22416dd99d5e430317c0a2bcd01fe83ffc2d7392da8',
4141
}),
4242
dict({
4343
'feature_key': 'downstream',
44-
'feature_spec_version': '8ba901bb7efebf6b75ab81334237508b6e399f5515d818e3f76a4cd6421991a4',
44+
'feature_spec_version': '2deaa627b51f493d71c80ae0a480c069077a5b1ea9876e55b51bc68f7e514800',
4545
'feature_version': '7b8e38a11f800fb93c18c0e04c4609d7b52ec12b62177ba8355531d363f01751',
4646
'snapshot_version': '1ab4ecd9d7af535b843dc22416dd99d5e430317c0a2bcd01fe83ffc2d7392da8',
4747
}),

tests/test_feature_metadata.py

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import json
2+
from typing import Any, cast
23

34
import pytest
5+
from pydantic import ValidationError
6+
from pydantic.types import JsonValue
47

58
from metaxy import Feature, FeatureKey, FeatureSpec
69
from metaxy.models.feature import FeatureGraph
@@ -43,31 +46,29 @@ class MetadataFeatureB(
4346

4447
def test_metadata_json_serializable() -> None:
4548
"""Validate JSON serialization enforcement for metadata."""
46-
valid_metadata = {
49+
valid_metadata: dict[str, JsonValue] = {
4750
"string": "value",
4851
"number": 42,
4952
"float": 3.14,
5053
"bool": True,
5154
"null": None,
52-
"list": [1, 2, 3],
53-
"nested": {"key": "value"},
55+
"list": cast(JsonValue, [1, 2, 3]),
56+
"nested": cast(JsonValue, {"key": "value"}),
5457
}
5558

5659
spec = FeatureSpec(
5760
key=FeatureKey(["tests", "metadata", "json"]),
5861
deps=None,
5962
metadata=valid_metadata,
6063
)
61-
assert spec.metadata is not None
62-
assert isinstance(spec.metadata, dict)
63-
assert isinstance(spec.metadata["list"], list)
64+
assert spec.metadata == valid_metadata
6465
assert json.dumps(spec.metadata) is not None
6566

66-
with pytest.raises(ValueError):
67-
FeatureSpec(
67+
with pytest.raises(ValidationError):
68+
_ = FeatureSpec(
6869
key=FeatureKey(["tests", "metadata", "json"]),
6970
deps=None,
70-
metadata={"func": lambda x: x},
71+
metadata=cast(Any, {"func": object()}),
7172
)
7273

7374

@@ -78,7 +79,16 @@ def test_metadata_immutable() -> None:
7879
deps=None,
7980
metadata={"key": "value"},
8081
)
81-
assert spec.metadata is not None
82+
assert spec.metadata == {"key": "value"}
8283

8384
with pytest.raises(Exception):
8485
spec.metadata = {"key": "new_value"} # type: ignore[assignment]
86+
87+
88+
def test_metadata_defaults_to_empty_dict() -> None:
89+
"""Metadata defaults to an empty dict for easier usage."""
90+
spec = FeatureSpec(
91+
key=FeatureKey(["tests", "metadata", "default"]),
92+
deps=None,
93+
)
94+
assert spec.metadata == {}

0 commit comments

Comments
 (0)