Skip to content

Honour force='yes' in stringify_param - #619

Open
ckarnell wants to merge 1 commit into
qlustered:masterfrom
ckarnell:fix-stringify-param-force
Open

Honour force='yes' in stringify_param#619
ckarnell wants to merge 1 commit into
qlustered:masterfrom
ckarnell:fix-stringify-param-force

Conversation

@ckarnell

Copy link
Copy Markdown

ChildRelationship.stringify_param takes force and never reads it, so path(force='yes') returns None for the case its own docstring calls out.

path() documents it like this:

Returns None if the path is not representable as a string. This might be the case ...
because custom objects used as dictionary keys (then there is a path but it's not representable).

:param force: If 'yes':
    Will return a path including '(unrepresentable)' in place of non string-representable parts.

A custom object used as a dictionary key:

class Id:
    def __repr__(self): return '<id>'
    def __hash__(self): return 1
    def __eq__(self, other): return isinstance(other, Id)

k = Id()
change = DeepDiff({k: 10}, {k: 20}, view='tree')['values_changed'][0]
change.path()             # None, correct
change.path(force='yes')  # None, documented as a path with '(unrepresentable)'

repr doesn't round-trip here, so stringify_param sets result = None and returns it whatever force says. NonSubscriptableIterableRelationship.get_param_repr does implement force, which is why the set case in test_non_subscriptable_iterable_path works and this one never has.

Three lines, in stringify_param where force already arrives.

One judgement call for you. The result goes through param_repr_format, so a dict key comes out as root[(unrepresentable)] while the existing set case is root(unrepresentable) because it bypasses the format. Brackets seemed right since the key is the part being replaced, but say the word if you'd rather have it raw. I left force='fake' alone.

Test is test_unrepresentable_dict_key_path, next to the set-case test. Suite goes 1258 to 1259, with the same 5 failures and 1 error before and after (they are in test_serialization.py and test_lfucache.py and predate this). Reverting only model.py fails the new test alone.

ChildRelationship.stringify_param accepted force and never read it, so
path(force='yes') returned None for an unrepresentable dict key, the case its
own docstring promises to render as '(unrepresentable)'. Adds
test_unrepresentable_dict_key_path.
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