aiida-core icon indicating copy to clipboard operation
aiida-core copied to clipboard

Add `flat` argument to `QueryBuilder.iterall()`

Open mbercx opened this issue 1 year ago • 4 comments

Is your feature request related to a problem? Please describe

Typical ways to obtain the results from a query are all(), first() (for testing), and iterall(). For larger queries, the latter is usually preferable since it doesn't first put all the nodes in a list like all (I believe).

all() and first() both have the input argument flat, which flattens the list and useful to e.g. loop over a query that only has one projection per result. However, it seems iterall() doesn't have this.

Describe the solution you'd like

Let's add a flat input argument to iterall() as well, it's more consistent and useful.

mbercx avatar Apr 24 '23 18:04 mbercx

I think I looked into doing this when implementing flat for all() and first() but vaguely remember that this wasn't possible or not trivial. Should maybe have a look before definitely assigning the good-first-issue label before sending someone on a wild-goose chase

sphuber avatar Apr 25 '23 13:04 sphuber

Righto! Fair point. 😅

I think I also noticed a nasty bug in iterall() last night. But have to recheck in a more clear state of mind. Will report back so if you go looking into this code we can take this into accounts.

mbercx avatar Apr 25 '23 14:04 mbercx

Side note: intuitively I would expect first() to always return with flat to True. Would it be sensible to make this the default?

mbercx avatar Apr 25 '23 14:04 mbercx

Side note: intuitively I would expect first() to always return with flat to True. Would it be sensible to make this the default?

Fully agree, but didn't do this because it would break backwards compatibility. Also didn't see a way for a deprecation pathway other than just warning if flat=False and say that default will become True in v3.0, but that would spam a lot of deprecation warnings by default since the default would now be False.

sphuber avatar Apr 25 '23 14:04 sphuber