Skip to content

fix(v1): route json() dumps kwargs to json.dumps, not dict() - #166

Merged
simontaurus merged 1 commit into
mainfrom
fix/v1-json-dumps-kwargs
Sep 24, 2026
Merged

simontaurus merged 1 commit into
mainfrom
fix/v1-json-dumps-kwargs

Conversation

@simontaurus

Copy link
Copy Markdown
Contributor

Regression since v1.0.0, reported downstream. LinkedBaseModel.json() of the v1 descriptor binding forwards every keyword to dict():

# src/oold/model/v1/_descriptor.py:342 at v1.0.2
encoder = kwargs.pop("encoder", None) or self.__json_encoder__
kwargs.pop("models_as_dict", None)
return json.dumps(self.dict(**kwargs), default=encoder)

pydantic v1 splits them: BaseModel.json() names the dict() arguments explicitly and collects the rest into **dumps_kwargs, which go to json.dumps. So a valid v1 call raises:

>>> p.json(exclude_none=True, exclude_defaults=True, indent=2)
TypeError: BaseModel.dict() got an unexpected keyword argument 'indent'

Same for sort_keys and separators. The legacy binding (OOLD_DESCRIPTOR_BINDING=0) is unaffected - src/oold/model/v1/__init__.py:723 keeps the explicit v1 signature.

Changes:

  • split the dumps kwargs off before calling dict(), keyed on the arguments BaseModel.dict accepts
  • add test_json_passes_dumps_kwargs_to_json_dumps to tests/test_compat_parity_v1.py; .json() had no coverage anywhere in the suite, which is how this shipped

734 passed under both bindings.

- indent, sort_keys and separators reached BaseModel.dict() and raised
  TypeError; pydantic v1 names the dict() arguments and collects the rest
- add the v1 parity test that covers the split
@github-actions

Copy link
Copy Markdown
Contributor

Release preview

Merging this PR would release v1.0.3 (current: v1.0.2).

Changelog preview (truncated)
## v1.0.3 (2026-09-24)

### Bug Fixes

- **v1**: Route json() dumps kwargs to json.dumps, not dict()
  ([`41b65de`](https://github.com/OO-LD/oold-python/commit/41b65ded09e9f25c5986179909e1d46df67bf805))

Preview via python-semantic-release and conventional commits.

@github-actions

Copy link
Copy Markdown
Contributor

📊 Benchmark Results

Click to see benchmark comparison
📊 Benchmark Comparison (threshold: 1.3x)
============================================================

⚠️  Performance Regressions:
  ❌ test_core[v2]: 0.0717s → 0.1207s (+68.4%, ratio: 1.68x)

✅ Performance Improvements:
  ✅ test_enum_docstrings: 0.0544s → 0.0405s (-25.6%, ratio: 0.74x)
  ✅ test_core[v1]: 0.0501s → 0.0370s (-26.2%, ratio: 0.74x)

➖ Unchanged (within threshold):
  ➖ test_simple_dict_document_store: 0.0012s → 0.0013s (+13.1%)
  ➖ test_sqlite_document_store: 0.0013s → 0.0014s (+7.7%)
  ➖ test_local_sparql_store: 0.0230s → 0.0228s (-0.8%)
  ➖ test_oneof_subschema: 0.0716s → 0.0617s (-13.9%)
  ➖ test_subclass_inheritance: 0.0518s → 0.0493s (-4.8%)
  ➖ test_class_hierarchy: 0.0572s → 0.0503s (-12.1%)
  ➖ test_schema_generation[v1]: 0.0010s → 0.0012s (+15.5%)
  ➖ test_schema_generation[v2]: 0.0023s → 0.0024s (+2.3%)
  ➖ test_simple_json: 0.0004s → 0.0004s (-1.4%)
  ➖ test_complex_graph: 0.0010s → 0.0009s (-5.9%)

============================================================
Summary: 1 regressions, 2 improvements, 10 unchanged
============================================================

⚠️  Regressions detected but not failing build (informational only)

Threshold: 1.3x (30% slower triggers a regression warning)

Note: Benchmarks are informational only and won't fail the build.

💡 Tip: Download the benchmark-results artifact for detailed JSON data

@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@simontaurus
simontaurus merged commit 1f8541c into main Sep 24, 2026
21 checks passed
@simontaurus
simontaurus deleted the fix/v1-json-dumps-kwargs branch September 24, 2026 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant