fix: update() with in-place callback returning None no longer overwrites field - #238
Open
deepakganesh78 wants to merge 1 commit into
Open
Conversation
…tes field (h2non#163) When a callable passed to update() modifies data in-place and returns None, Fields._update_base and Index._update_base now preserve the in-place modification instead of overwriting the field with None. Both in-place (returning None) and return-value callbacks are supported. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #163
Problem
Since commit 7987969 (
Fix issue with lambda based updates),Fields._update_baseassigns the return value of the callable back todata[field]:python data[field] = val(data[field], data, field)This broke callbacks that modify
data[field]in-place and returnNone(the pre-7987969 pattern that issue #163 relies on), becauseNoneoverwrites the value the callback just set.Index._update_basehad the opposite inconsistency — it called the callable without assigning the return, so return-value callbacks were silently ignored for array indices.Root cause
Neither
Fields._update_basenorIndex._update_basehandled both callback styles (in-place and return-value).Fix
Both methods now capture the callable's return value and only assign it if it is not
None. If the callable returnsNone(explicitly or implicitly), the in-place modification is preserved.Nonevia a returning callbackThis fix means a callback like
lambda orig, data, field: Nonecan no longer be used to deliberately set a field toNonevia its return value. No existing test or documented behavior relies on this pattern, and it is the only reasonable way to restore compatibility with the in-place callback contract that existed before 7987969 and that issue #163 depends on. Callers who need to setNonecan still do so inside the callback body:data[field] = None.For
Index._update_base, this is a net improvement — return-value callbacks (e.g.lambda orig, data, idx: orig + 1) now work for array indices, where previously the return value was silently discarded.Reproduction
`python
from jsonpath_ng import parse
def lowercase_inplace(orig, data, field):
data[field] = data[field].lower()
# returns None
data = {'Data_cat': {'data_entry': [
{'value': 0, 'UPPERCASE': 'UPPERCASE_A'},
{'value': 2, 'UPPERCASE': 'UPPERCASE_B'},
]}}
parse('$..UPPERCASE').update(data, lowercase_inplace)
Before fix: UPPERCASE fields become None
After fix: UPPERCASE fields become lowercase strings
`
Validation
$..fielddoubledot notation)lambda x, y, z: x + 1) exercisesFields(notIndex) and continues to pass