cliqz-oss / keyvi

Keyvi - a key value index that powers Cliqz search engine. It is an in-memory FST-based data structure highly optimized for size and lookup performance.
https://cliqz.com
Apache License 2.0
179 stars 38 forks source link

[RFC] Subpackages for pykeyvi #233

Open hendrikmuhs opened 7 years ago

hendrikmuhs commented 7 years ago

For a new feature I like to discuss modularization. I like to put the new feature into a subpackage ('pykeyvi.index'). Playing a bit with cython it unfortunately turns out, cython does not support it easily:

https://github.com/cython/cython/wiki/FAQ#how-to-compile-cython-with-subpackages

There is a recommendation on the wiki:

https://github.com/cython/cython/wiki/PackageHierarchy

which would build a 'so' for every subpackage, which would mean a lot of wasted space and higher memory usage.

RFC: Move the cython extension into a hidden namespace ('pykeyvi._core') keeping the flat structure, create a python module structure and import accordingly, this would keep a single so file but still provide subpackages:

 - pykeyvi
  - __init__.py
  - index
   - __init__.py

pykeyvi/init.py:

from pykeyvi._core import JsonDictionaryCompiler, ...

pykeyvi/index/init.py:

from pykeyvi._core import IndexReader, IndexWriter

In the longer run this also solves 2 problems:

hendrikmuhs commented 7 years ago

@narekgharibyan

narekgharibyan commented 7 years ago

@hendrikmuhs sounds good! Have you tried to implement it already, to see how it works with packaging ?