Issue/639 allow iteration over dict keys #9882
Conversation
| if isinstance(value, list): | ||
| _iterator = value | ||
| elif isinstance(value, dict): | ||
| _iterator = value.keys() # type: ignore |
There was a problem hiding this comment.
Not sure how to type this
There was a problem hiding this comment.
I see what you did here, but I don't feel that we should propagate the dicts this deeply. For one thing, dicts aren't always flattenable. While in the DSL we have no nested lists, we can have dicts inside lists. In other words [[1], [2]] is equivalent to [1, 2], but [{"a": "b"}] is not equivalent to {"a": "b"} or to ["a"].
I can imagine it may be difficult to understand the full implications of this, considering that this is quite an advanced method in a codebase you're only partially familiar with. The model below might help illustrate it. When compiled on master, it prints @{'a': 1, 'b': 2}. On your branch, it prints that, preceded with a and b. The fact that it prints both is actually due to the lack of a consistency safeguard in the for loop. The list comprehension does have it. If such a safeguard where there, the compiler would raise an exception instead. Either way, this is clearly not the intention.
d = {"a": 1, "b": 2}
for i in [d]:
std::print(i)
end
All this said, what would be the appropriate location to handle this? I was going to say For.execute() , which is mostly just a type check, and GradualFor.receive_result. But as I type this I realize that GradualFor.receive_result will also need some way to determine whether it should or should not iterate any dict values it receives. And I don't have an immediate solution for it. I'll get back to this tomorrow.
There was a problem hiding this comment.
I'm starting to think that this will be a major hurdle, and that we should go for the plugin after all. We can discuss it tomorrow.
|
@sanderr Let me know if I missed anything it's been a while since I worked on the DSL |
Description
Add iteration over dict keys in for loops
closes inmanta/std#639
Self Check:
Strike through any lines that are not applicable (
~~line~~) then check the box