Warn users about failed cdef class lookups#3846
Warn users about failed cdef class lookups#3846da-woods wants to merge 4 commits intocython:masterfrom
Conversation
One of the most commonly seen user errors is to try to access cdef methods/attributes of a cdef class when Cython doesn't know the type. This attempts to add warnings to alert the user so that they can hopefully correct it themselves. It obviously isn't (and can't be) comprehensive but does catch the obvious cases. It's possible that it'll be a bit noisy/over-sensitive. Hopefully the test-suite should give an indication of how many false positives it generates.
b46663c to
bed2d3b
Compare
|
This is generating a few warnings when compiling the Cython source code. But at least some of them look like they might be legitimate... |
scoder
left a comment
There was a problem hiding this comment.
I left a few comments, but overall, I'm torn. It's nice to help users understand their programming errors, but it's annoying for users to fight false positives. I fear that it might do more harm than good.
| else: | ||
| self.python_attribute_lookup_warnings(env) |
There was a problem hiding this comment.
The if not … and not … above feels like we should swap the if-clause with the else.
Also, we probably shouldn't get here for the is_error case either, right?
| else: | ||
| self.python_attribute_lookup_warnings(env) | ||
|
|
||
| def python_attribute_lookup_warnings(self, env): |
There was a problem hiding this comment.
This method sometimes uses return and sometimes return None. The latter seems more appropriate (given that it otherwise returns an entry), but since the only usage ignores the return value, I wonder what the goal here is.
I do see the point. A few thoughts:
|
|
If there is a "mostly safe" subset of cases, then we could warn about those. Otherwise, I'd rather leave this out. |
|
At the moment it seems like this will always generate too many false positives (and there doesn't seem much value in having it but setting it off by default, since it's really targeted at beginners and not the people who've read the manual for the obscure compiler options). I'll close it for now. If I come up with a more targetted way of doing it then I may have another look in future. But probably not. |
One of the most commonly seen user errors is to try to access cdef methods/attributes of a cdef class when Cython doesn't know the type.
This attempts to add warnings to alert the user so that they can hopefully correct it themselves. It obviously isn't (and can't be) comprehensive but does catch the obvious cases.
It's possible that it'll be a bit noisy/over-sensitive. Hopefully the test-suite should give an indication of how many false positives it generates.