Skip to content
This repository has been archived by the owner on Dec 11, 2023. It is now read-only.

introduction of an abstraction layer for the "results array" #187

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ARF1
Copy link

@ARF1 ARF1 commented May 5, 2015

Implicitly closes #66 and #176 by allowing users to "bring their own" implementations.
Would benefit from #189.

This introduces an abstraction layer for the generation of the "results array" in ctable.__getitem__() and for data access.

It has minor impact on core bcolz code and minimizes overhead using lookup optimizations:

Without abstraction layer:

In [3]: a = bcolz.open(rootdir='testdata.bcolz')

In [4]: %timeit a[1]
10000 loops, best of 3: 38.5 µs per loop

With abstraction layer:

In [3]: a = bcolz.open(rootdir='testdata.bcolz')

In [4]: %timeit a[1]
10000 loops, best of 3: 42.3 µs per loop

bcolz.ctable._outstruc_fromindices = value.fromindices
bcolz.ctable._outstruc_fromboolarr = value.fromboolarr
assert hasattr(value, '__setitem__')
except AttributeError, AssertionError:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parenthesis are needed for Python 3

@FrancescAlted
Copy link
Member

I need to think a bit more on this, but at least initially this seems like a good idea to me.

@ARF1 ARF1 force-pushed the out_structure_abstraction_layer branch from b15ba87 to a8fee6c Compare May 5, 2015 19:55
@ARF1 ARF1 mentioned this pull request May 6, 2015
@ARF1 ARF1 force-pushed the out_structure_abstraction_layer branch from a8fee6c to 8ff5422 Compare May 6, 2015 13:10
This allows implementations of the abstraction layer to cache their data
structures just like we currently do for numpy
@ARF1 ARF1 force-pushed the out_structure_abstraction_layer branch from 8ff5422 to f5bcd90 Compare May 6, 2015 13:23
ARF added 2 commits May 6, 2015 15:51
* tuples are created from the list of fields in a row only in the numpy
implementation of the abstraction layer. Other implementation might be able to
use the list instead without the expense of the conversion.
* use list instead of set for the `weakref.WeakSet` work-around
@esc
Copy link
Member

esc commented May 23, 2015

Tests?

@FrancescAlted
Copy link
Member

FrancescAlted commented Feb 1, 2017

I think that this would definitely be a nice addition. We still need tests and update the documentation so that user "may bring their own implementation". @ARF1 could you please do that?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider adding a Categorical datatype to bcolz
3 participants