8000 assert_equal gives KeyError: 0 for some Python objects. · Issue #5285 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

assert_equal gives KeyError: 0 for some Python objects. #5285

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.

Sign up for GitHub

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

Jump to bottom
Closed
mdickinson opened this issue Nov 17, 2014 · 8 comments
Closed

assert_equal gives KeyError: 0 for some Python objects. #5285

mdickinson opened this issue Nov 17, 2014 · 8 comments

Comments

@mdickinson
Copy link
Contributor

In NumPy 1.9, numpy.testing.assert_equal can raise a KeyError in some cases. An example is below. It's not as minimal as I would like: I was unable to easily remove the dependence on the Traits library.

from UserDict import DictMixin
from numpy.testing import assert_equal
from traits.api import HasTraits


class DataContext(DictMixin, HasTraits):
    def __init__(self):
        self.subcontext = {}

    def __getitem__(self, key):
        return self.subcontext[key]

    def __setitem__(self, key, value):
        self.subcontext[key] = value

    def keys(self):
        return self.subcontext.keys()


# This code works.
a = DataContext()
a['foo'] = 2
a['bar'] = 3
assert_equal(a, a)

# But this doesn't: the call to `assert_equal` gives `KeyError: 0`.
c = DataContext()
c['foo'] = 1
assert_equal(c, c)

The above code fails for me under NumPy 1.9 (but not NumPy 1.8), with the following output:

(.devenv)taniyama:Desktop mdickinson$ python numpy_bug.py 
Traceback (most recent call last):
  File "numpy_bug.py", line 31, in <module>
    assert_equal(c, c)
  File "/Users/mdickinson/Enthought/Source/meta-geophysics/.devenv/lib/python2.7/site-packages/numpy/testing/utils.py", line 282, in assert_equal
    usecomplex = iscomplexobj(actual) or iscomplexobj(desired)
  File "/Users/mdickinson/Enthought/Source/meta-geophysics/.devenv/lib/python2.7/site-packages/numpy/lib/type_check.py", line 269, in iscomplexobj
    return issubclass(asarray(x).dtype.type, _nx.complexfloating)
  File "/Users/mdickinson/Enthought/Source/meta-geophysics/.devenv/lib/python2.7/site-packages/numpy/core/numeric.py", line 462, in asarray
    return array(a, dtype, copy=False, order=order)
  File "numpy_bug.py", line 13, in __getitem__
    return self.subcontext[key]
KeyError: 0

I have no idea why the array constructor would be trying to access item 0 of this object, but I can imagine that there might be valid reasons for doing so. If so, it might be useful to turn the except ValueError for the use_complex section of the assert_equal code into except (KeyError, ValueError). (Code here).
@juliantaylor
Copy link
Contributor

that type pretends to be a sequence while its a mapping, a recent optimization in discover_dimension causes the issue.
As it was supported before it should be adapted to handle this again.

@mdickinson
Copy link
Contributor Author

Interesting; thanks for the pointer to discover_dimensions. I haven't yet understood why the addition of the HasTraits base class makes the result of PySequence_Check true. (Without the inheritance from HasTraits, there's no issue here.)

@abalkin
Copy link
Contributor
abalkin commented Nov 18, 2014

#5063 is possibly related to this.

@juliantaylor
Copy link
Contributor

yes its a related issue, though the fix is not the same (probably need to either revert or special case list/tuple), this object even has a len. I was trusting the testsuite that does cover the keyerror exception, but not in this way :(

@njsmith
Copy link
Member
njsmith commented Nov 18, 2014

Numpy's assert_equal is only documented to work on arrays and built-in
containers, but I guess there's an argument for handling arbitrary python
objects. Maybe if the clever container comparisons raise an error, we
should fall back on just trying a simple == check?
On 18 Nov 2014 18:12, "Julian Taylor" notifications@github.com wrote:

yes its a related issue, though the fix is not the same, this object even
has a len. I was trusting the testsuite that does cover the keyerror
exception, but not in this way :(

—
Reply to this email directly or view it on GitHub
#5285 (comment).

@seberg
Copy link
Member
seberg commented Nov 6, 2021

Modifying the code to not inherit and adding a __len__ still works again. The array-coercion code changed a lot in 1.19 (or was it 1.20?), so I am willing to be the situation is changed and probably is back to what it was before.

Happy to look into it, but it seems to me that it is probably fixed and the old reproducer doesn't work anymore.

@seberg seberg closed this as completed Nov 6, 2021
@mdickinson
Copy link
Contributor Author

Thanks, @seberg. Agreed that it doesn't make sense to keep this open. I'd still like to understand the combination of factors that led to this, but I think if anything that belongs in a Traits issue rather than here.

@seberg
Copy link
Member
seberg commented Nov 6, 2021

Well, the old numpy code had three different paths to walk the nested sequences (dimension discovery, dtype discovery, and filling the array) it is/may be confronted to in np.asarray. I guess one of them behaved differently. My guess (but too lazy to check) is that one of them just called GetItem assuming that if we got that far, it must be a proper sequence.

The new code uses PySequence_Fast() exactly once, so I expect things are fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@seberg @abalkin @juliantaylor @njsmith @mdickinson
0