bidict should be subclass of dict about bidict HOT 17 CLOSED

Perhaps implement __subclasshook__.

from bidict.

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.

from bidict.

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.

from bidict.

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!

from bidict.

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).

from bidict.

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!)

from bidict.

I suspect the use case is fairly common.

from bidict.

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.

from bidict.

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

from bidict.

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.

from bidict.

@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.

from bidict.

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.

from bidict.

@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?

from bidict.

@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".

from bidict.

@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.

from bidict.

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.

from bidict.

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.

from bidict.