fix type annotation

[kovaacs]

The type annotation is wrong here, it is expected to be a tuple

[erezsh]

Thanks! You're right, but I believe that the fix should be in line 146

    if isinstance(key_columns, str):
        key_columns = (key_columns,)
    else:
        key_columns = tuple(key_columns)

(str is a valid input too)

[kovaacs]

Right, I missed that. Changed to allow string as well.

[kovaacs]

Ah, sorry, I missed your point. I guess any iterable of strings OR string should be accepted, then the code you copied will convert it to a tuple.

[erezsh]

Yes, sounds good.

…

On Fri, Jan 10, 2025 at 17:40 Marcell Kovács ***@***.***> wrote: Ah, sorry, I missed your point. I guess any iterable of strings OR string should be accepted, then the code you copied will convert it to a tuple. — Reply to this email directly, view it on GitHub <#58 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFSSSEFD5P43JPDNDI24YD2J7ZZJAVCNFSM6AAAAABU6V7LPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOBTGIYTSNZSHE> . You are receiving this because you commented.Message ID: ***@***.***>

[kovaacs]

I also adjusted connect_to_table() to match the behaviour of diff_tables()

[erezsh]

Sorry, my code should have been key_columns = tuple(key_columns or ()) or similar

[kovaacs]

I also missed that key_columns is optional, maybe I should also make that clearer with the annotation

[erezsh]

Ok, I think None has a special meaning, and converting None to () messes it up down the line.

Sorry, it's been a while since I worked on this code, so I forgot the nuances.

If you prefer, I can try to finish the PR myself. But you're of course welcome to do it yourself.