OpenMS / autowrap

Generates Python Extension modules from Cythons PXD files
Other
75 stars 22 forks source link

special method __getitem__ #13

Closed hroest closed 9 years ago

hroest commented 9 years ago

I see that there is a special __getitem__ method called create_special_getitem_method but I think there is an a slight logical problem here. In our case in OpenMS it generates code such as

    def __getitem__(self,  index ):
        assert isinstance(index, (int, long)), 'arg index wrong type'

        if (<size_t>index) < 0:
            raise IndexError("invalid index %d" % (<size_t>index))
        if (<size_t>index) >= self.inst.get().size():
            raise IndexError("invalid index %d" % (<size_t>index))
        cdef _DataFilter * _r = new _DataFilter(deref(self.inst.get())[(<size_t>index)])

which does not make a lot of sense here to compare a <size_t>index < 0 . The code is generated by a file called src/pyOpenMS/pxds/DataFilters.pxd which looks like this

 cdef extern from "<OpenMS/FILTERING/DATAREDUCTION/DataFilters.h>" namespace "OpenMS":

     cdef cppclass DataFilters "OpenMS::DataFilters":
         DataFilters() nogil except +
         DataFilters(DataFilters) nogil except + #wrap-ignore
         DataFilter operator[](Size index) nogil except + # wrap-upper-limit:size()

other classes that use int instead of Size do not have this issue.

In conclusion, I dont think it is a big problem but might be worthwhile to consider in the future

hroest commented 9 years ago

after trying out the newest version, I get the following issue

Error compiling Cython file:
------------------------------------------------------------
...
        self.inst = shared_ptr[_DataFilters](new _DataFilters())

    def __getitem__(self,  index ):
        assert isinstance(index, (int, long)), 'arg index wrong type'

        cdef long _idx = in_0
                            ^
------------------------------------------------------------

pyopenms/pyopenms.pyx:38650:29: undeclared name not builtin: in_0
created pyopenms.cpp
copied files needed for distribution to pyopenms/

could this have something to do with 458c55a9eb7c3ec03c40de4d69aaf317e9eba1fe maybe?

uweschmitt commented 9 years ago

Fixed this with 502c039187d095acd1a1b1819aa5e472efc9f080