Alias `inverse` for `inv` please? about bidict HOT 6 CLOSED

@jab Thank you! I appreciate you hearing me out and taking my perspective on the matter into account, and appreciate the work that you've done both to get this change implemented and released and to create and maintain the bidict package in general.

from bidict.

My pleasure, and thank you for contributing this feedback!

ICYMI, these changes have now been released in v0.18.0.

from bidict.

Thanks for the suggestion, @mentalisttraceur!

I've considered this in the past, but wanted to wait for a user to actually ask for it before considering it further. And now that time has come!

I'm still considering, but to help decide I've worked up a PR for this in #87. Would you like to give it a review? One thing to weigh in on is: for __subclasshook__’s purposes of whether to consider some unknown, third-party Mapping type a (virtual) subclass of BidrectionalMapping, an inverse attribute is still strictly optional; only an inv attribute is required:
https://github.com/jab/bidict/blob/invalias/bidict/_abc.py#L102

P.S. No pressure, but I just invited you to join the bidict chat in case it's of interest. I'd love to hear more about how and where you use bidict, how well it's working for you, etc. The chat room is extremely low-activity, but if you join, you can get an early heads up when a new release is coming out, weigh in on any proposed changes, etc. Hope to see you there!

from bidict.

Initial thoughts:

The project's README contains the following section right at the beginning, which attempts to document the centerpiece that is the .inv attribute as simply, loudly, and clearly as possible:

Quick Start

>>> from bidict import bidict
>>> element_by_symbol = bidict({'H': 'hydrogen'})
>>> element_by_symbol['H']
'hydrogen'
>>> element_by_symbol.inv['hydrogen']
'H'

So the first thing I want to confirm is whether that's fulfilling its intended purpose well enough.

If so, then next I would say: This README not only powers the content in https://github.com/jab/bidict#readme and https://bidict.readthedocs.io, but (to your point about stuff on the web being out-of-band) is moreover distributed inside the package, to make it available to users for offline reference (along with all the other docs).

So the README is not out-of-band while working at the command line with the package installed (even if working offline). But it is out-of-band while working inside an interactive repl session. In that case, what if this 4-line Quick Start example were just added to the bidict module docstring, so it appeared as part of the help(bidict) output? Would that suffice?

That said, since the technical risk here seems low, and the benefit seems at least slightly higher, I'm inclined to not push back too hard on this, but ideally I'd get some feedback from other users first. Perhaps @lordmauve (who quite rightly proposed dropping the old ~ alias for getting the inverse way back in #19 (though in that case readability was clearly worse, and in the .inverse case it's better)) can throw in his £.02?

from bidict.

@jab I think it's a great idea to update docstrings so that help() outputs show inv early on. That definitely helps with discoverability.

But I don't want us to forget my first point, which help text doesn't really address: I'd like it if people who read my code, including future me, could see the .inverse and not have to spend mental cycles deducing what it was.

Actually, I just remembered another equally big one: searchability. I've found over the last few years that having full words available in the code helps tremendously with searching, in two ways:

1. Full words are easier to think of and remember: when a piece of code abbreviates, I have to try to remember what that specific project chose as their abbreviation, and I don't always remember correctly. If I haven't thought about it for half a year, I can remember "inverse" or even "it was some word like 'reverse' but not exactly, but I think it had the 'verse' ending, so I'll grep for verse" a lot easier than I can remember "inv".

2. Full words reduce false positives. Imagine I have the string invalid all over my code in various identifiers, or another member of my team abbreviated "invites" to inv somewhere else: a search for inv or .inv or even .inv[ might produce a lot of matches I don't need. If I can use the long form, searching becomes more powerful.

I also agree that inverse should (at least for now) remain strictly optional for __subclasshook__: if we made it mandatory this would turn it into a backwards-incompatible change - we don't want to break (virtual) subclass detection for people (if we did want to make it mandatory later, I think that can be handled the way you'd handle any other interface compatiblity breaking change).

from bidict.

@mentalisttraceur I decided to go all in: In #87 I've now renamed BidirectionalMapping.inv to BidirectionalMapping.inverse, made BidirectionalMapping.__subclasshook__ now require an inverse attribute instead of inv (taking advantage of the major version 0 to make the technically breaking change[0]), and added the inv alias to BidictBase. This is how I wish I'd done it in the first place.

[0] I've never actually heard of any users actually using BidirectionalMapping.__subclasshook__, i.e. using isinstance(foo, bidict.BidirectionalMapping) where foo is some bidict-compatible third-party bidirectional mapping. (FWIW I found a sentry.utils.datastructures.BidirectionalMapping but this actually has an inverse attribute, not an inv attribute, so the new bidict.BidirectionalMapping.__subclasshook__ is now compatible with it, for better or worse: the semantics of the sentry BidirectionalMapping's inverse are different (it's a method, not a property, and returns a copy, so mutating calls like sentry_bimap.inverse().pop(bar) are very surprising if you're used to some_bidict.inv.pop(bar).)

from bidict.