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

TypeError with NoneType from dict #2152

Closed
jetuk opened this issue Mar 15, 2018 · 1 comment
Closed

TypeError with NoneType from dict #2152

jetuk opened this issue Mar 15, 2018 · 1 comment

Comments

@jetuk
Copy link

jetuk commented Mar 15, 2018

I think there is a regression in 0.28 where by the following code no longer works. I have tested this in 0.27.3 and it is fine. I can't see anything obvious in the changelog that would suggest this is now not supported. It appears to be something to do with a change to how type checking is done after the call to pop.

cdef class MyClass:
    cdef public MyClass attr
    def __init__(self, **kwargs):
        self.attr = kwargs.pop('attr', None)

def main():
    A = MyClass()  # This doesn't work
    B = MyClass(attr=A)

The error is as follows:

Traceback (most recent call last):
  File "run_cython.py", line 2, in <module>
    main()
  File "cython_ext_type_none.pyx", line 12, in cython_ext_type_none.main
    A = MyClass()  # This doesn't work
  File "cython_ext_type_none.pyx", line 7, in cython_ext_type_none.MyClass.__init__
    self.attr = kwargs.pop('attr', None)
TypeError: Cannot convert NoneType to cython_ext_type_none.MyClass

Cython 0.27.3 produces the following C

  /* "cython_ext_type_none.pyx":7
 * 
 *     def __init__(self, **kwargs):
 *         self.attr = kwargs.pop('attr', None)             # <<<<<<<<<<<<<<
 *         #self.attr = attr
 * 
 */
  __pyx_t_1 = __Pyx_PyObject_GetAttrStr(__pyx_v_kwargs, __pyx_n_s_pop); if (unlikely(!__pyx_t_1)) __PYX_ERR(0, 7, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_1);
  __pyx_t_2 = __Pyx_PyObject_Call(__pyx_t_1, __pyx_tuple_, NULL); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 7, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_2);
  __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0;
  if (!(likely(((__pyx_t_2) == Py_None) || likely(__Pyx_TypeTest(__pyx_t_2, __pyx_ptype_20cython_ext_type_none_MyClass))))) __PYX_ERR(0, 7, __pyx_L1_error)
  __Pyx_GIVEREF(__pyx_t_2);
  __Pyx_GOTREF(__pyx_v_self->attr);
  __Pyx_DECREF(((PyObject *)__pyx_v_self->attr));
  __pyx_v_self->attr = ((struct __pyx_obj_20cython_ext_type_none_MyClass *)__pyx_t_2);
  __pyx_t_2 = 0;

Whereas Cython 0.28 produces the following. __pyx_t_2 has gone entirely and there is no test for Py_None as far as I can tell.

  /* "cython_ext_type_none.pyx":7
 * 
 *     def __init__(self, **kwargs):
 *         self.attr = kwargs.pop('attr', None)             # <<<<<<<<<<<<<<
 *         #self.attr = attr
 * 
 */
  __pyx_t_1 = __Pyx_PyDict_Pop(__pyx_v_kwargs, __pyx_n_s_attr, Py_None); if (unlikely(!__pyx_t_1)) __PYX_ERR(0, 7, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_1);
  if (!(likely(__Pyx_TypeTest(__pyx_t_1, __pyx_ptype_20cython_ext_type_none_MyClass)))) __PYX_ERR(0, 7, __pyx_L1_error)
  __Pyx_GIVEREF(__pyx_t_1);
  __Pyx_GOTREF(__pyx_v_self->attr);
  __Pyx_DECREF(((PyObject *)__pyx_v_self->attr));
  __pyx_v_self->attr = ((struct __pyx_obj_20cython_ext_type_none_MyClass *)__pyx_t_1);
  __pyx_t_1 = 0;

For reference the following does work. Note the intermediate local variable attr inside __init__.

cdef class MyClass:
    cdef public MyClass attr
    def __init__(self, **kwargs):
        attr = kwargs.pop('attr', None)
        self.attr = attr

def main():
    A = MyClass()  # This does work.
    B = MyClass(attr=A)
@scoder
Copy link
Contributor

scoder commented Mar 18, 2018

Thanks for the excellent test case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants