[#243] Fix expand query parameter #286
Conversation
SonnyBA
force-pushed
the
issue/#243-fix-expand-param
branch
from
03821ea to
8e27f8e
Compare
SonnyBA
force-pushed
the
issue/#243-fix-expand-param
branch
from
8e27f8e to
6137dd5
Compare
| if child.many: | ||
| results.setdefault(child.label, []).append(child_result) | ||
| child_results = results.setdefault(child.label, []) |
There was a problem hiding this comment.
Am I wrong or is child_results not used for anything? We populate the list but we don't return it or append it to results
There was a problem hiding this comment.
There was a problem hiding this comment.
The setdefault call for the results dictionary ensures that the child.label key exists with the value of a list. It also returns said list which will be used later on (if child_result has a value set).
There was a problem hiding this comment.
If I get this right, you are mutating the results because child_results has a reference to it?
There was a problem hiding this comment.
Could you make this a bit more readable? Because this does make it quite difficult to comprehend what is happening.
There was a problem hiding this comment.
I didn't remove the setdefault usage as I think it handles various things through a single method call (setting the value if it is not present, retrieving its content). I did add some type hints in case people are not familiar with the method. The nested if statements were also removed to make things more readable.
| data = None | ||
| id = str(uuid4()) |
There was a problem hiding this comment.
Is there a reason why we need to fill id when there is no object? From what I can quickly see locally this isn't necessary.
Fixes #243
Changes
Shows given expand query parameter results even when these are empty or
null.