Merge master into enh/cython-port #294
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
the fvalidate method
changed download_url in setup.py to point to github repo
Passing in None will trigger a check for kwargs == NULL in
methodobject.c:PyCFunction_Call, causing a TypeError on the
Cython side ("TypeError: _name_default() takes no keyword arguments")
Cython fills in tp_dictoffset with 0, since a cdef class has no __dict__ by default. This is a problem for PyObject_GenericGetAttr, which consults this member to find the object's dict during attribute lookup.
When the call to PyObject_GenericGetAttr fails for an attribute lookup, NULL is returned and PyErr is set. When getattr_python finishes, None is returned and Python terminates, since attribute lookup resulted in an value (None) but with the error flag set. Perhaps there's a better way of "raising" the exception on the Python side, but the only way I could get this to work was by clearing the error, and manually re-raising it in Cython.
PR #124 switched the use of the deprecated .set method to .trait_set, since the former is not defined on the Cython side. However, a later commit undid that change, causing the tests to fail.
The `value` object may be unhashable, in which case a TypeError is raised. In the original C implementation this was done with a PyDict_GetItem(..., value) == NULL, but whereas PyDict_GetItem swallows all errors, we need to be more careful.
The C implementation has a method validate() and a property validate. To support both on the Cython side I renamed the property to validate_ and implemented the method.
__delattr__ now calls _internal_setattr with a special NullObject value to indicate delete. This mimics the C implementation and the code to handle the NullObject case was already there.
This was referenced May 22, 2016
Fixes for the cython port
|
@jvkersch it sounds like this one will be be merged into |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Does nothing but merge master into the enh/cython-port branch.