Skip to content

fix(wtsite): repair the dangling combine_into reference - #156

Merged
LukasGold merged 1 commit into
mainfrom
fix/wtpage-update-dict
Sep 2, 2026
Merged

fix(wtsite): repair the dangling combine_into reference#156
LukasGold merged 1 commit into
mainfrom
fix/wtpage-update-dict

Conversation

@LukasGold

Copy link
Copy Markdown
Contributor

Closes #15.

Changes

  • _combine_into in src/osw/wtsite.py: the recursive dict merge that update_dict has always called and that never existed.
  • WtPage.update_dict is now a staticmethod and delegates to it. It never used self.
  • WtPage.set_value: dropped print(match.full_path) and called the merge helper directly.
  • tests/test_wtpage_update_dict.py: 10 offline tests.

Rationale

update_dict called wt.combine_into, which does not exist anywhere in this repository and never has. It was written as WtPage.combine_into in the commit that introduced the class (5486996, October 2022) and was never defined, then later rewritten to wt.combine_into without being defined there either. Any nested dict therefore raised AttributeError.

Writing tests for the one caller, set_value, turned up two further defects on the same path:

  • WtPage.update_dict(match.value, value) at the call site passed two arguments to a three-parameter instance method, so match.value bound to self and update was missing. TypeError before the missing reference was ever reached.
  • print(match.full_path) raised TypeError: expected string or bytes-like object, got 'int'. set_value keys its working dict by list index, and jsonpath_ng regex-matches field names when it renders a path, so rendering an integer key fails. This one broke set_value on every call, replace=True included, which is why the other two were never observed.

All three are fixed here rather than filed separately, since they are one broken path and the tests that exposed them are in this PR.

Notes

  • update_dict and set_value keep their @deprecated markers. This repairs them, it does not revive them.
  • Making update_dict a staticmethod keeps the existing WtPage.update_dict(a, b) call shape working and does not break page.update_dict(a, b) either. The three-argument form was never functional.
  • A dict merged onto a non-dict replaces it, and a fresh nested dict is copied rather than aliased into the merge target, so a later edit of the result cannot reach back into the argument.
  • print(match.full_path) was debug output, so it is removed rather than converted to logging. A comment records why the path cannot be rendered, since that is not obvious. Converting the library's remaining 205 print calls is replace all print statements with proper logging #130.
  • The deeper option the issue raises, reworking WtPage._dict for multi-slot pages, is untouched. That is a different change and these methods are deprecated.

Verification

Offline suite passes (64 passed, 1 skipped).

- add _combine_into, the recursive dict merge update_dict always called
- make update_dict a staticmethod, matching how set_value calls it
- drop print(match.full_path), which raised TypeError on integer keys
- cover all three with offline tests
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Release preview

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

Changelog preview (truncated)
## v2.0.3 (2026-09-02)

### Bug Fixes

- **wtsite**: Repair the dangling combine_into reference
  ([`707be40`](https://github.com/OpenSemanticLab/osw-python/commit/707be4030e9221e8180de0198e4282b4d0cb4fcc))

### Testing

- Rename oold.py to oold_test.py so its tests are collected
  ([`20072a9`](https://github.com/OpenSemanticLab/osw-python/commit/20072a9249cd97126a222c62a70f84e0433343ef))

Preview via python-semantic-release and conventional commits.

@LukasGold LukasGold left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed missing reference correctly

@LukasGold
LukasGold merged commit 1dc554f into main Sep 2, 2026
12 checks passed
@LukasGold
LukasGold deleted the fix/wtpage-update-dict branch September 2, 2026 16:27
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.

fix: missing reference combine_into

1 participant