Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Rename Associative to AbstractDict #25012

Merged
merged 1 commit into from
Dec 14, 2017
Merged

RFC: Rename Associative to AbstractDict #25012

merged 1 commit into from
Dec 14, 2017

Conversation

andyferris
Copy link
Member

@andyferris andyferris commented Dec 10, 2017

I mentioned this at the "API consistency review" issue #20402 (comment). I wanted to see how this one would play out - it wasn't too hard in the end.

Some of the reasons I think this is worthwhile:

  • To me the smoking gun is we have type families AbstractArray, AbstractSet, AbstractRange and AbstractString with prototypical concrete types Array, Set, Range, String. It seems consistent to also have AbstractDict and Dict. EDIT: Range was recently renamed AbstractRange, in the name of consistency.
  • Associative is a bit of a unique case - all concrete implementations are named Dict, SortedDict, WeakKeyDict - and I haven't seen a SortedAssociative or ImmutableAssociative in the wild (someone please correct me if I'm wrong).
  • It will aid new users in navigability, who will explore the type system by jumping from Foo to AbstractFoo and back again. They should be able to guess that a Dict is an AbstractDict, when they first come across AbstractDict in a function signature. They will have one less word to learn!
  • I've had conversations with Julia programmers at work who didn't know what Associative was. It's quite possible that many new users with a non-computer-science background don't recognize the term "associative array" (I hadn't, before Julia - I did know of maps, dictionaries, hash tables... even "a functional relation of two-tuples"... so I wasn't completely naive).
  • To me at least, I saw many places where the docstrings and code became a tiny bit clearer in this PR. It felt almost relieving to describe in English such containers as "dictionaries" rather than "associative collections".
  • Associative is an adjective. Types in Julia are usually nouns. (OK, OK, but we have had bikesheddings over less). The word Association (used by at least Mathematica) would be seem more correct than Associative (though I wouldn't recommend that change over AbstractDict).
  • This is probably the last time to consider this change.

(Apart from the bikeshedding issues, this also implicitly touches on some dicussions about the "meaning" of Associative collections, because generally our AbstractFoo / Foo pairs behave quite similarly. In particular, refer to comments like this and the second point in this (the basic summary is: should the Associative interface allow or disallow multi-maps, etc, which are a generalized kind of associative container - my position is that it is very useful to know that the "dictionary" interface guarantees that the keys appear only once in the collection, and that other abstract types or data structures could and should be used to model multi-maps instead))

The changes in this PR are purely cosmetic (no change in functionality).

@andyferris andyferris added the collections Data structures holding multiple items, e.g. sets label Dec 10, 2017
@innerlee
Copy link
Contributor

From math background, I always thought Associative must be some object associated with the associative law. The change is more clear in expressing meaning.

@rfourquet
Copy link
Member

rfourquet commented Dec 10, 2017

I was uncomfortable with the unclarity that an Associative can contain non-unique keys, so I find it great to clarify AbstractDict -> unique keys. So why not after all keep Associative mean the general thing, and have Dict <: AbstractDict <: Associative, and e.gl Base.ImmutableDict <: AbstractMultiDict <: Associative ?

@rfourquet
Copy link
Member

Oh, for the record, there isn't a prototypical concrete Range type ;-)

@kmsquire
Copy link
Member

I really like @rfourquet’s suggestion, which would allow the type hierarchy in DataStructures.jl to be filled out properly. Right now, neither MultiDict nor SortedMultiDict subclass Associative, although they clearly share many of its properties.

@ararslan ararslan added the triage This should be discussed on a triage call label Dec 10, 2017
@andyferris
Copy link
Member Author

Yes, having an Associative supertype would be a nice idea.

However, I'd point out that (a) adding an immediate supertype to AbstractDict should not be breaking for v1.x (someone please correct me if I misunderstand this), and (b) I like to think of abstract types as defining interfaces so I'd like to understand what methods the Associative supertype is expected to provide. (In terms of interfaces, I'm looking forward to multiple inheritance or similar, so that we can break down functionality a bit more precisely).

Oh, for the record, there isn't a prototypical concrete Range type ;-)

lol, my bad.

@StefanKarpinski
Copy link
Member

I'm fine with either renaming Associative to AbstractDict or inserting AbstractDict as a more specific subtype of Associative – as you see fit. There will be some shaking out of what exactly the distinction is over time, but that can evolve outside of Base as long as the types exist.

@andyferris
Copy link
Member Author

Can I request that triage please discuss whether to include Associative here as the supertype of AbstractDict, or to remove completely (I'm happy to do the work to put it back in as a supertype).

My point of view is that adding things with no concrete types (or even defined interface) is not going to be helpful to many people, and that adding a supertype in later when we know exactly how that helps would be a better approach. A deprecation in v0.7 marks a fresh start - my worry is that without this, there may be code out there that expects e.g. unique keys from Associative that may remain as Associative instead of specifying AbstractDict.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Dec 13, 2017

Doing a simple rename for now is the better approach. I am a one-man triage team 👅

@andyferris andyferris removed the triage This should be discussed on a triage call label Dec 14, 2017
@andyferris andyferris added this to the 1.0 milestone Dec 14, 2017
@andyferris andyferris merged commit 3af34b3 into master Dec 14, 2017
@StefanKarpinski StefanKarpinski deleted the ajf/abstractdict branch December 14, 2017 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collections Data structures holding multiple items, e.g. sets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants