Expose get_all method in Multimap interface and provide factory method for Multimap
I attempted to implement fix for osmcode/pyosmium#66 but I'm missing few constructs in libosmium.
- Factory methods for
Multimaps. There are few approaches that I considered, but neither is without drawbacks:
- just copy&paste most of
MapFactoryto Multimap - create generic
MapFactorythat can register and create bothMapandMultimapby extending the template with one more argument, but this will break backward compatibility - create generic
MapFactoryas a "meta-template", and create separate factories forMapandMultiMap
-
Multimapinterface doesn't specifyget_allmethod (not all implementing classes implement this) so there is no way to retrieve data usingMultimapinterface, user would have to bind to specific implementation -
(probably not that important)
MultimapandMapmissiterator,begin,endtriple to make it easy to expose a way to iterate through the (key, value) pairs -
multimap/all.hppis missingmultimap/hybrid.hpp#include
The Multimaps are somewhat neglected area in Osmium. They are not well documented, not well tested, and, as you noticed, incomplete. You have to know exactly what you are doing to use them correctly and efficiently. I am not sure it even makes sense to expose the Multimaps in a generic way to Python. Maybe it would be better to expose only a specific one or just a few of them.
With the current libosmium I recommend using the FlexMem map for almost all use cases, maybe we need a FlexMultiMap first and can just expose that. In any case I'd want more tests before I would start to refactor or extend this code.
To your points:
- I think just copying and then changing the
MapFactorywould be the easiest approach. We definitely want to keepMapFactoryandMultiMapFactoryseparate things. But, as mentioned above, I am not convinced yet that we even need the factory. - I can't remember why
get_allisn't implemented in some classes. We'd have to check whether this is possible at all and how to handle the case when it isn't implementable. - We have to see whether this can be implemented in all classes.
- I am not sure the
all.hppis that useful. Maybe we should deprecated use of that file.