BUG: check_api_dict does not correctly handle tuple values

[eric-wieser]

It seems that at some point we augmented the indices with extra data. But this
data should not be included when determining uniqueness.

Previously, this would crash if there actually was a collision, rather than
printing a useful message

[njsmith]

Is it just me, or did the original code make no sense at all...? If we did have fully duplicate indices, wouldn't that be silently discarded before we ever made it into this function?

[eric-wieser]

I ended up finding this because the code was detecting a duplicate index, but then crashed when trying to tell me which one it was. So the old code did work, and clearly didn't go with the transition to (ind,) over ind

[charris]

Might look better if this was broken into a fmt and val, and then ValueError(fmt.format(val)) or some such.

[eric-wieser]

Problem is, the format is sort of contained in val as well:

fmt = "Same index has been used twice in api definition: {}"
val = ''.join(
    '\n\tindex {} -> {}'.format(index, names)
    for index, names in doubled.items() if len(names) != 1
)
raise ValueError(fmt.format(val))

I don't think that fmt and val are useful names here, because both contain format strnigs

[eric-wieser]

I guess I could do:

dupes = {index: names for index, names in doubled.items() if len(names) != 1}

raise ValueError("Same index has been used twice in api definition: {}".format(
    ''.join(
        '\n\tindex %d -> %s' % (index, names)
        for index, names in dupes.items()
    )
)

Although this is really bikeshedding, and I don't really want to switch working tree again...

[charris]

Nothing wrong with bikeshedding, code clarity is part of the process...

[eric-wieser]

Do you have a preference between the one in this PR and the two alternatives above?

[charris]

I rather like the first. Probably put the closing ) for val at the end of the preceding line there.

[charris]

LGTM, maybe a style nit.

[charris]

Need to finish this up.

[eric-wieser]

@charris: Done

[eric-wieser]

@charris: Ping - ball is in your court now

[charris]

Tnanks Eric.