ShadowRanger
ShadowRanger

Reputation: 155536

Are non-member operator overloads (specifically operator==) broken in Cython?

I've got a use case I'm testing where, for performance, it's convenient to write a Cython wrapper class for C++'s unordered_map<int, int> type up front, then pass the wrapper to a bunch of Cython functions that use it, rather than always passing around Python dict (keyed and valued by Python ints that fit in C int) and converting on the fly over and over. I'm also trying to bake in some basic functionality to make it behave like a dict at the Python layer, so it can be worked with without converting en masse from Python dict to Cython unordered_map<int, int> wrapper and back.

My problem occurs in trying to implement __eq__ efficiently in the simplest case (comparing one wrapper instance to another). AFAICT, in theory Cython's libcpp.unordered_map pxd definition pretends unordered_map has a member operator==. This answer claims it should be fine, even though the operator== is actually a non-member function, since the definition only needs to exist so Cython knows it can just put a literal == in the code; the C++ compiler will look up the real overload when compiling the extension module.

But in practice, I can't seem to get it to work without additional manual (and hacky) intervention. For experimenting, I've just been using ipython %%cython magic. Right now what I'm doing is (minimized as much as possible while still exhibiting problem):

>>> %load_ext cython
>>> %%cython -+ --verbose --compile-args=/std:c++latest
... from libcpp.unordered_map cimport unordered_map
... from cython.operator cimport dereference as deref
... cdef extern from "<unordered_map>" namespace "std":
...     bint operator==(unordered_map[int, int]&, unordered_map[int, int]&)
... cdef class UII(object):
...     cdef unordered_map[int, int] c_map
...     def __cinit__(self, dict py_map):
...         cdef int k, v
...         for k, v in py_map.iteritems():
...             self.c_map[k] = v
...     def __eq__(UII self, other):
...         if isinstance(other, UII):
...             return self.c_map == (<UII>other).c_map
...         return NotImplemented
...

To be clear, this works right now, e.g.:

>>> pydict = {1:2, 3:4}; ui = UII(pydict); uieq = UII(pydict); uine = UII({1: 2, 4: 3})
>>> ui == uieq   # Compares equal for same value UIIs
True
>>> ui == uine   # Correctly determines different valued UII not-equal
False

but it only works because I included:

cdef extern from "<unordered_map>" namespace "std":
    bint operator==(unordered_map[int, int]&, unordered_map[int, int]&)

to explicitly define the non-member overload of operator== for the <int, int> template specifically (because Cython doesn't support generically templated functions, only generically templated classes and explicitly declared templates of functions). If I omit those lines, I get the following error from Cython:

[1/1] Cythonizing C:\Users\ShadowRanger\.ipython\cython\_cython_magic_ea9bfadf105ac88c17e000476fd582dc.pyx

Error compiling Cython file:
------------------------------------------------------------
...
        cdef int k, v
        for k, v in py_map.iteritems():
            self.c_map[k] = v
    def __eq__(UII self, other):
        if isinstance(other, UII):
            return self.c_map == (<UII>other).c_map
                             ^
------------------------------------------------------------

C:\Users\ShadowRanger\.ipython\cython\_cython_magic_ea9bfadf105ac88c17e000476fd582dc.pyx:13:30: Invalid types for '==' (unordered_map[int,int], unordered_map[int,int])

indicating that it believes there is no overload for operator== available. Am I doing something wrong that I can fix, or is Cython broken for non-member operator== (and possibly other non-member functions, despite what this answer claims)? I hate explicitly defining every template specialization for non-member functions; it's not a huge burden for this specific case, but it seems odd that Cython would define member overloads, presumably to solve this specific problem, but not actually be able to use them. I checked the actual text of unordered_map.pxd, and it's definitely defined there:

cdef extern from "<unordered_map>" namespace "std" nogil:
    cdef cppclass unordered_map[T, U]:
        # ... irrelevant stuff expunged ...
        bint operator==(unordered_map&, unordered_map&)  # Looks properly defined...

Upvotes: 1

Views: 406

Answers (1)

DavidW
DavidW

Reputation: 30909

Two things are broken about Cython's handling of non-member operators:

  1. Non-member operators defined in a .pxd file outside a class aren't correctly cimported to Cython (the workround is to do from something cimport *) and thus aren't used. This is what I think I said in my previous answer

  2. Non-member operators defined within a class with two arguments (as in the code you showed in unordered_map.pxd) aren't recognised by Cython and aren't used (despite being defined like that all over the C++ standard library wrappers included in Cython). At one point I tried to submit a patch for this but it was ignored. This is no longer true. Only point 1 now applies.

What does work is to tell Cython that it's a member operator (even if C++ implements it as a non-member operator). Therefore a simple patch to unordered_map.pxd would work. Note that I'm changing it to be defined with one argument and the C++ implicit this/self:

cdef extern from "<unordered_map>" namespace "std" nogil:
    cdef cppclass unordered_map[T, U]:
        # ... irrelevant stuff expunged ...
        bint operator==(unordered_map&)

Alternatively, you can define it yourself before you need to use it (like you're doing currently) but as a template. This at least saves you having to define every specialization

cdef extern from "<unordered_map>" namespace "std":
    bint operator==[R,S](unordered_map[R, S]&, unordered_map[R, S]&)

i.e. the statement in your question

(because Cython doesn't support generically templated functions, only generically templated classes and explicitly declared templates of functions)

isn't true.


It is all a bit of a mess though

Upvotes: 1

Related Questions