Skip to content
This repository has been archived by the owner on Jul 7, 2022. It is now read-only.

Change method of checking if a key is in the dictionary. #36

Merged
merged 1 commit into from
Aug 27, 2020

Conversation

budude2
Copy link
Contributor

@budude2 budude2 commented Jun 17, 2020

The original method would return false if the key contained the boolean "false" causing the value to not be deleted even if it existed.

The original method would return false if the key contained the boolean "false" causing the value not to be deleted even if it existed.
@russellballestrini
Copy link
Owner

Could you add a failing test that this fixed to prevent future regressions?

@budude2
Copy link
Contributor Author

budude2 commented Jun 18, 2020

Sure, let me put one together really quick.

@budude2
Copy link
Contributor Author

budude2 commented Jun 18, 2020

Here's the JSON code:

{
    "test1":
    {
        "integerTest": 10,
        "floatTest": 3.4,
        "boolTrue": true,
        "boolFalse": false,
        "arrayTest": [10, 20, 30, 40],
        "arrayTest2": [2.2, 3.14, 5.23, 6.28]
    },

    "test2":
    {
        "integerTest2": 24,
        "floatTest2": 6.28,
        "boolTrue2": true,
        "boolFalse2": false,
        "arrayTest3": [60, 70, 80, 90],
        "arrayTest4": [1.24, 6.78, 3.42, 8.90]
    }
}

When I run it before hand:

>>> pprint.pprint(nested_lookup.nested_delete(configDict, 'boolFalse'))
{'test1': {'arrayTest': [10, 20, 30, 40],
           'arrayTest2': [2.2, 3.14, 5.23, 6.28],
           'boolFalse': False,
           'boolTrue': True,
           'floatTest': 3.4,
           'integerTest': 10},
 'test2': {'arrayTest3': [60, 70, 80, 90],
           'arrayTest4': [1.24, 6.78, 3.42, 8.9],
           'boolFalse2': False,
           'boolTrue2': True,
           'floatTest2': 6.28,
           'integerTest2': 24}}

When I run it after:

>>> pprint.pprint(nested_lookup.nested_delete(configDict, 'boolFalse'))
{'test1': {'arrayTest': [10, 20, 30, 40],
           'arrayTest2': [2.2, 3.14, 5.23, 6.28],
           'boolTrue': True,
           'floatTest': 3.4,
           'integerTest': 10},
 'test2': {'arrayTest3': [60, 70, 80, 90],
           'arrayTest4': [1.24, 6.78, 3.42, 8.9],
           'boolFalse2': False,
           'boolTrue2': True,
           'floatTest2': 6.28,
           'integerTest2': 24}}

@grenzi
Copy link
Contributor

grenzi commented Aug 25, 2020

just to chime in here, the same issue exists (and would be resolved by this PR) if the keys value is falseish, e.g., 0

>>> d = {"x":0, "y": 12, "z":3}
>>> nested_delete(d, 'x')
{'x': 0, 'y': 12, 'z': 3}
>>> nested_update(d, 'x', 1, in_place=True)
>>> nested_delete(d, 'x')
{'y': 12, 'z': 3}

@russellballestrini
Copy link
Owner

@grenzi could you take a pass at adding this as a unit test?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants