Skip to content

Refactor JSON Patch OpRemove and OpMove - #137

Open
jg-rp wants to merge 1 commit into
mainfrom
op-remove
Open

jg-rp wants to merge 1 commit into
mainfrom
op-remove

Conversation

@jg-rp

@jg-rp jg-rp commented Jul 28, 2026 •

Copy link
Copy Markdown
Owner

This PR refactors OpRemove and OpMove to use a common _remove() function for consistency and better code reuse, just like we did with _add() in #136.

OpReplace does not use _remove() due to its distinct behavior when replacing a JSON root value.

@carlitogwapito

Copy link
Copy Markdown

A small optional test-only commit, based directly on this branch.

The shared helper now rejects a move from a MappingProxyType or tuple. The old implementation writes the destination without removing anything from those read-only sources, so it behaves like a copy.

Two small guards worth keeping: reject the read-only source, and leave the destination unchanged on that rejection. This doesn't assume apply() is atomic for other failures.

Checked on Python 3.14.4: both new cases pass your head and catch the old copy-like behavior; the existing move-to-root and array-append tests pass on both. Focused Ruff lint/format passes. I haven't run the full Hatch/compliance suites, typing, build or optional-regex/version/OS matrix.

To run the added file in your tree: python -m pytest tests/test_move_readonly_source.py -q.

This branch has not been deployed

No deployments
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.

2 participants