[RFC] ParentsProvider returning a dictionary instead of a list

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

John Arbash Meinel wrote:
> Aaron Bentley wrote:
>> One of the reasons I originally went with a list was that I wanted to
>> make it very clear when a requested set of parents was not being
>> returned (e.g. due to not-present revisions).  How do you plan to
>> represent that?  Not putting the child revision in the dictionary?
>> That's easy to miss.  Assigning the value to None?  That's also easy to
>> miss if you're using dict.get().

> I went with not present in the dict.

I would like to avoid making this API tricky to use, and I think
silently returning fewer values than expected makes it tricky.

> Because it makes _StackedParentsProvider
> fairly trivial:
> 
> parents = {}
> needed = set(revisions)
> for provider in providers:
>   found = provider.get_parent_map(needed)
>   parents.update(found)
>   needed.difference_update(found)
>   if not needed:
>     break
> return parents

Oh, but using None would also be trivial:

parents = dict((r, None) for r in revisions)
for provider in providers:
    needed = set(r for r, v in parents.iteritems() if r is None)
    if needed == set():
        break
    parents.update(parentsprovider.get_parent_map(needed))

And the same algorithm would work if we used a reserved id like
"missing:" instead of None.

> Having a dictionary also means that people who want to:

I'm not opposed to having a dictionary, I just want to make the
interface as direct and unsurprising as possible.

> "if X is not there, give me None" can use "dict.get". For people who want more
> direct checking "if X not in dict".

> I understand the desire to iterate using iteritems(), however, that won't ever
> go in a sorted order (for some uses that is important). And regardless, doing:
> 
> for revision, parents in izip(revisions, get_parents(revisions)):
> 
> didn't seem better than
> 
> parent_map = get_parent_map(revisions)
> for revision in revisions:
>   parents = parent_map.get(revision)

Well, there is a difference to me in that the first form will tell us
that some data was missing, and the second form just skips it.

> The graph code I was working on does *not* do that. Because it batches up the
> requests across all searchers. So it does get_parents() on a larger set.
> 
> 
> The other *big* reason to use a dict, is because all of the current users of it
> *want* a dict. So they end up doing:

I repeat, I am not opposing a dict interface.  I'm merely trying to
avoid a surprising interface.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHXrRx0F+nu1YWqI0RAlS6AJ9/bT+JqBlFNFxt8/Ho8z0e4jT2wwCfcubE
gL0jFuYRLCZyFbYE2ck3HKY=
=QxAr
-----END PGP SIGNATURE-----