Dictionary should expose its index values

[trxcllnt]

Per @TheNeuralBit's feedback, DictionaryVector should add a method for retrieving index values.

I tend to think of the dictionary index as the sorted keys list, so dictionary.key(i) feels right to me (and name feels complementary to dictionary.get(i)). but it's possible this name is too limiting/generic/non-descriptive.

@TheNeuralBit has proposed dictionary.getEncoded(i) which also works, but typically I'd expect a method named getEncoded to retrieve raw bytes from a Utf8/List Vector?

Comments welcome.

[TheNeuralBit]

Yeah I agree that getEncoded can be confusing. I also called the function for returning a value given a key decode, which is consistent... but has the same flaw.

dictionary.key(i) sounds good - any thoughts on an analogous name for decode? Maybe dictionary.value(k)?
Just to be clear I'm referring to a function such that dictionary.get(i) === dictionary.value(dictionary.key(i))

[trxcllnt]

@TheNeuralBit That works for me. It's worth mentioning that I've been using designs from an earlier internal columnar storage project as inspiration. I didn't declare these interfaces up front since it seems like all Arrow Vectors are of the form <T, number>, but I'm happy to add them if you or @wesm think it would be worthwhile:

/**
 * Vector stores values by position.
 * If Pos = number, Vector can be a dense Array<Val>
 */
interface Vector<Val, Pos> {
    get(pos: Pos): Val;
}

/**
 * Index maps keys to vector positions.
 * The simplest implementation is a dictionary.
 * If Key = Pos, Index is an identify function.
 * Index can be also defined as a Vector<Pos, Key>, but a standalone definition makes adding features later easier
 */
interface Index<Key, Pos> {
    get(key: Key): Pos;
}

/**
 * A Column is a pair of (Index<Key, Pos>, Vector<Val, Pos>).
 */
interface Column<Val, Key, Pos> {
    get: (key: Key) => Val;
    getAt: (pos: Pos) => Val;
    keyAt: (pos: Pos) => Key;
}

In terms of the above definitions, does this API look OK to you?

class DictionaryVector<T> implements Column<T, number, number> {
    protected data: Vector<T, number>;
    protected keys: Index<number, number>;
    /* Dictionary `get` is pandas `iloc`, yeah? */
    get(i) { return this.getAt(this.keyAt(i)); }
    getAt(position) { return this.data.get(position); }
    keyAt(position) { return this.keys.get(position); }
}

[wesm]

You could also call them indices for consistent terminology, e.g. dict_vector.index(i) -- in C++ dict->indices() returns the indices as an integer array. I don't have a strong preference. It is true that one frequently operates on the indices independent from the dictionary values -- e.g. it's much faster to do analytics on integers than strings.

[trxcllnt]

@wesm I'm down w/ that

[trxcllnt]

@TheNeuralBit ok! commit a8341f5 adds dict.index(i) and dict.value(i) methods, and shipped in v0.1.2

[TheNeuralBit]

@trxcllnt sorry for not engaging on this for a while, I'm just now getting around to trying out the new code and I'm struggling a bit. It seems to me that maybe StructVector (and DictionaryVector for #6) should be exported? Otherwise how could I access the index and value methods?

I'm definitely no TypeScript expert, so maybe I'm missing something... here's an example of what I'm trying to do:

import {Table, Vector, DictionaryVector} from 'apache-arrow';

var table: Table = Table.from(...);
var col: Vector<any> = table.getColumnAt('dictionary');

if (col instanceof DictionaryVector) {
    // fun things with col.index and col.value
} else {
    // :(
}

We can take this conversation over to the apache repo or jira if need be, just thought I'd re-engage here for continuity

[trxcllnt]

@TheNeuralBit ah sorry, yeah we need to export the type definitions. I forget b/c we use the lib in JS, where the types don't matter. Will submit a patch to the apache project and publish a release after it's merged

[wesm]

Can you open a JIRA? I see that a8341f5 added some things but there are no tests, so we should also add some tests for this while we're at it

[trxcllnt]

@wesm yep, will do

[trxcllnt]

@wesm I created an issue here: https://issues.apache.org/jira/browse/ARROW-1544, PR here: apache/arrow#1103