Fix tabulate col size in case of empty cell

[McSinyx]

Previously, the size is no less than len(str(None)) == 4.

This commit also add type hint and docstring to the function.

Edit: I'm not sure if this change is trivial, please tell me if news is required.

[xavfernandez]

Do we agree that this case never happens in pip's code base ?
Each column has at least an header with a length > 4.
(if that is indeed the case, it means that this change is not user facing and qualifies as "trivial")

A simple parametrized test would also be a nice touch 👼

[McSinyx]

Do we agree that this case never happens in pip's code base? Each column has at least an header with a length > 4.

Sorry I did not give the context, I was preparing for GH-7975 and there might be (I don't think it's settled) some narrow columns. Additionally, should we move the function to utils.misc?

A simple parametrized test would also be a nice touch 👼

That won't be a problem!

[xavfernandez]

Moving it to utils.misc is a good idea if you plan to use it for pip find/discover (and don't forget the tests ;) )

[uranusjr]

If I’m not mistaken, the problem here is basically len(str(None)) == 4, right? Would it be good enough to add type hints so the caller always passes in an empty string, not None?

Scratch that, I see the problem here is zip_longest returning None by default. The fix (suppling fillvalue) makes sense to me.

[McSinyx]

That was the problem, which will be fixedby this PR. Consider the current implementation:

sizes = [0] * max(len(x) for x in vals)
for row in vals:
    sizes = [max(s, len(str(c))) for s, c in zip_longest(sizes, row)]

None comes from zip_longest, and I'm not aware of an elegant way to hint dynamic length of multiple iterables (i.e. the rows). Plus in this PR tries to simplify the logic which at that time was probably more of a proof of concept.

^{Fly away writing some tests for this after a full day of long and hard lectures.}

Edit: Just want to point out how long it takes me to respond that uranusjr found the anwser himself 😆

[pradyunsg]

Trivial is fine, since there's not really going to be any signficant user facing changes here IMO.

[McSinyx]

Since the change is quite trivial (pun intended), may I get this merged now?

[xavfernandez]

Thanks @McSinyx