ENH: skip or avoid gc/objectmodel differences btwn pypy and cpython #7912
Conversation
| return - other | ||
| def __add__(self, other): | ||
| return other | ||
| sys.getrefcount = lambda x: AlwaysTrue() |
There was a problem hiding this comment.
I'm not sure Numpy should monkeypatch sys on import.
Probably simpler to guard the few asserts with if not IS_PYPY
There was a problem hiding this comment.
Yeah, monkeypatching the standard library sounds evil ;)
|
Fixed as per comments |
| @@ -6355,50 +6360,52 @@ def test_string(self): | |||
| assert_equal(np.where(False, b, a), "abcd") | |||
|
|
|||
|
|
|||
| class TestSizeOf(TestCase): | |||
| if not IS_PYPY: | |||
There was a problem hiding this comment.
Another way to do this in Nose is probably to raise SkipTest in def setup(self): ...
There was a problem hiding this comment.
Interesting suggestion. Probably not as obvious in reading the code, but otherwise cleaner, at least until we drop the now abandoned Nose.
There was a problem hiding this comment.
Using dec.skip() in a setup() class method did not skip the class. It seems to skip one or more tests but not all. I could not find documentation or examples of how to do this in the 10 minutes I tried.
|
LGTM, @charris merge? |
| @@ -34,6 +34,7 @@ def iter_iterindices(i): | |||
| i.iternext() | |||
| return ret | |||
|
|
|||
| @np.testing.dec.skipif(not HAS_REFCOUNT, "python does not have sys.getrefcount") | |||
There was a problem hiding this comment.
Should be @dec.skipif(not HAS_REFCOUNT, "python does not have sys.getrefcount"), dec is imported above. That also brings the line length under the 79 character limit...
|
Looks straight forward. The only nit I see is that in the tests |
|
Thanks for the reviews. I will update the pull request accordingly |
|
Thanks @mattip . |
three classes of fixes when running tests with PyPy:
sys.getsizeof()sys.getrefcount()is used by defining a hackish AlwaysTrue class