bidict should be subclass of dict

[vojtechpachol]

issubclass(bidict, dict) returns False. We thus cannot pass it to function that test it. If dict were at least at last place of mro it would be settled.

[Twangist]

Perhaps implement __subclasshook__.

[jab]

Thanks for raising this issue, @pacholik. The thing is, bidict actually isn't a subclass of dict, it's an implementation of collections.Mapping. By checking for that your code will be more polymorphic:

>>> from collections import Mapping
>>> from bidict import bidict
>>> b = bidict()
>>> isinstance(b, Mapping)
True

I could understand the argument that being able to check isinstance(b, dict) would be more convenient, but if it's not actually necessary or true, would this be compromising purity for the sake of convenience, or is this actually an omission?

Let me know what you think, and meanwhile I'll give the collections.abc docs another read, and if it's still not clear, appeal to Python-dev etc. for more expert guidance.

[Twangist]

I'd say, then, no need to allege via __subclasshook that issubclass(bidict, dict) because I think it's probably better for user code to check issubclass(bidict, Mapping) (or, more likely, isinstance(thing, Mapping) as opposed to isinstance(thing, dict)) -- and evidently bidict already does the right thing.

[jab]

Cool, in that case just closed this with a documentation clarification. @pacholik, please let me know if this doesn't adequately address your use case.

In general, I always appreciate meeting new bidict users (thanks for stopping by:), hearing about your use cases, and any other ways bidict could better meet them. Please let me know anything you'd like to share!

[Twangist]

You're welcome, and thanks for a good package. I've used it most recently to biject between database column names which aren't valid Python identifiers, and SQLAlchemy names for the same columns which are (valid Python identifiers).

[jab]

Interesting, hadn't encountered this use case, thanks for sharing! I wonder if this is common. If so maybe I'll ask SQLAlchemy if they'd accept a PR adding a tip to their docs about this? /cc @zzzeek (ps hey @zzzeek!)

[Twangist]

I suspect the use case is fairly common.

[zzzeek]

um. we're looking for what, some kind of recipe w/ bidict? would need a lot of specifics for that use case. SQLA lets you put a .key on a Column if you want to deal with some friendly python name, you never need to know the real col names at all, so a use case here is not apparent to me.

[jab]

Thanks for the info @zzzeek! I don't have experience with SQLA so will let @Twangist respond but am interested in following along.

[Twangist]

In my use case, I have a table T in a Postgres db which inherits from a base table containing common fields such as creator, updated-by, timestamps. It was convenient to have a class attribute holding the collection of fields unique to T (in fact, most but not all of those, even). In the db, the fieldnames use "kebob-case", and in a few places I need to translate those names to SQLA colnames and vice-versa. A bidict provides both the collection and the translation capability.

[vojtechpachol]

@jab , @Twangist

Well, I know I can check for issubclass(bidict, Mapping), thing is, I'm not the one who is checking.

In particular df.rename checks if columns arg is instance of dict and if not, it expect a function. If I pass bidict, it fails with mybidict is not callable.

Anyway, thank you for your time.

[Twangist]

Aha. OK then maybe df.rename can be improved. Note that Its doc is ambiguous: first it says that it expects a "function / dict", but then it says in the parameters section "index, columns : dict-like or function, optional". Evidently "dict-LIKE" is false. So there's either a df.rename bug or a documentation bug -- consider reporting it. You're usng latest/greatest Pandas? (I assume)

For now, you could convert your bidict to a dict when calling df.rename -- df.rename(dict(my_bidict)) or somesuch accommodation -- but would get onerous if you do it alot.

[jab]

@pacholik @Twangist +1 on this being an issue on Pandas's side.

@zzzeek What did you think of @Twangist's reply to you 3 comments back?

[zzzeek]

@jab it sounds vague and very specific to some part of someone's application without any context, so far not much material for the stated goal of a "documentation tip".

[jab]

@pacholik @Twangist Since it looks like this wasn't reported upstream yet, I just submitted pandas-dev/pandas#11461 in case you're interested in following there.

[jab]

Also, I ended up asking about this on the python-dev list, and https://mail.python.org/pipermail/python-dev/2015-October/142058.html suggests that bidict's current approach is sound.

[jab]

One other reason to not subclass dict is we would inherit the fromkeys classmethod, which makes no sense for a bidict, and so would have to go out of our way to remove it.