Fix path behaviour for set_ #214
Conversation
src/pydash/objects.py
Outdated
| if isinstance(token, PathToken): | ||
| key = token.key | ||
| default_factory = pyd.get(tokens, [idx + 1, "default_factory"], default=default_type) | ||
| else: | ||
| else: # pragma: no cover |
There was a problem hiding this comment.
If the else case is no longer valid and isinstance(token, PathToken) is always true, then I think we could remove the if-statement and just have whatever is under the main if section.
Unless there was a reason to leave them in?
There was a problem hiding this comment.
Yeah makes sense. I removed them.
I left them just to be safe, but I guess if it happens it would probably be a rare edge case. The test cases cover a lot of cases and every passes so it should be safe enough.
There was a problem hiding this comment.
would probably be a rare edge case
From looking at the code, the tokens are from to_path_tokens() which always returns a PathToken list, right? So should never be any other type. Unless I missed something in to_path_tokens() where a different type could be included?
There was a problem hiding this comment.
to_path_tokens can return something else than a PathToken if it is a single element like "0" or "some_key".
But I guess we should probably make to_path_tokens always return a list of PathToken all the time. It would be more consistant and easier to reason about.
I added a commit with it. Let me know what you think
|
Lint fails in CI on files I didn't touch and everything passes localy. Not sure what's going on. Maybe we can just try rerunning it? |
Latest version of black made some changes to the formatting and required some flake8 ignores on certain rules. Rebase with |
DeviousStoat
force-pushed
the
fix-set_
branch
from
1f391a1 to
99a8a4d
Compare
fixes #213
The default type when setting a missing key is usually given by the
PathTokenbuilt into_path_tokens.However this function is incomplete and only builds a list of
PathTokens if the path is a string.The issue here is that that line sets the default type to
listif the object to update is not adict.And as stated above, when we have a path as a list
to_path_tokensjust returns the list and we don't have any information about the default factory, we just default tolist.So when we create the first list and try to update it we expect an index.
This PR updates the
to_path_tokensfunction to be more general and build the list ofPathTokeneven for a list.The added
pragma: no coverare there because this case should now be impossible to reach, the only case where the tokens are notPathTokens is when the path is only 1 non string value and we go directly to the end case as the for loop only iterates on the initial values.