Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@staticmethod with implicit object argument type is handled wrongly #3174

Open
navytux opened this issue Oct 10, 2019 · 3 comments

Comments

@navytux
Copy link
Contributor

@navytux navytux commented Oct 10, 2019

Hello up there. Please consider the following example:

---- 8< ---- moda.pxd

cdef class MyClass:
    @staticmethod
    cdef static_func(x)

---- 8< ---- moda.pyx

cdef class MyClass:
    @staticmethod
    cdef static_func(x):
        return x+1

---- 8< ---- modb.pyx

from moda cimport MyClass

def test():
    x = MyClass.static_func(2)
    print(x)

When compiled and run it fails with:

$ python -c 'import modb; modb.test()'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "modb.pyx", line 4, in modb.test
    x = MyClass.static_func(2)
TypeError: Cannot convert int to moda.MyClass

If I add explicit object to x argument in moda.pxd:

--- a/moda.pxd
+++ b/moda.pxd
@@ -1,3 +1,3 @@
 cdef class MyClass:
     @staticmethod
-    cdef static_func(x)
+    cdef static_func(object x)

It works:

$ python -c 'import modb; modb.test()'
3

Thanks beforehand,
Kirill

/cc @robertwb

navytux added a commit to navytux/cython that referenced this issue Oct 10, 2019
…gument type

See cython#3174.
If I remove `# doctest: +SKIP`, the test fails as:

    FAIL: call_static_pxd_cdef_with_implicit_object (line 102) (static_methods.__test__)
    Doctest: static_methods.__test__.call_static_pxd_cdef_with_implicit_object (line 102)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "/usr/lib/python2.7/doctest.py", line 2224, in runTest
        raise self.failureException(self.format_failure(new.getvalue()))
    AssertionError: Failed doctest test for static_methods.__test__.call_static_pxd_cdef_with_implicit_object (line 102)
      File "/home/kirr/src/tools/py/cython/TEST_TMP/run/c/static_methods/static_methods.so", line unknown line number, in call_static_pxd_cdef_with_implicit_object (line 102)

    ----------------------------------------------------------------------
    File "/home/kirr/src/tools/py/cython/TEST_TMP/run/c/static_methods/static_methods.so", line ?, in static_methods.__test__.call_static_pxd_cdef_with_implicit_object (line 102)
    Failed example:
        call_static_pxd_cdef_with_implicit_object(2)
    Exception raised:
        Traceback (most recent call last):
          File "/usr/lib/python2.7/doctest.py", line 1315, in __run
            compileflags, 1) in test.globs
          File "<doctest static_methods.__test__.call_static_pxd_cdef_with_implicit_object (line 102)[0]>", line 1, in <module>
            call_static_pxd_cdef_with_implicit_object(2)
          File "tests/run/static_methods.pyx", line 108, in static_methods.call_static_pxd_cdef_with_implicit_object
            return FromPxd.static_cdef_with_implicit_object(x)
        TypeError: Cannot convert int to static_methods.FromPxd

/cc @robertwb
@navytux

This comment has been minimized.

Copy link
Contributor Author

@navytux navytux commented Oct 10, 2019

I forgot to mention that it fails for both current 0.29.x and master.
I added corresponding test in #3175.

@scoder

This comment has been minimized.

Copy link
Contributor

@scoder scoder commented Oct 12, 2019

My guess is that it fails because @staticmethod is not evaluated (correctly) in .pxd files. Or maybe it incorrectly overwrites the prior handling of the decorator in the implementation file somehow.

PR for that is welcome, but I can't say what needs to be done for that. Needs some analysis to find out where this should be fixed.

@navytux

This comment has been minimized.

Copy link
Contributor Author

@navytux navytux commented Oct 13, 2019

@scoder, thanks for feedback. My impression is that somehow the "pass" that automatically annotates first argument of a method of a cdef class with corresponding type overwrites @staticmethod effect somehow. But I did not looked into the code at all. For now I filed PR with test (#3175, it can be already merged), this issue, so that the bug is not forgotten, and in my codebase worked around the problem with explicitly using object type for the first argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.