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

properly diff objects with arrays as attributes on variables #9169

Merged
merged 10 commits into from
Jun 30, 2024

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Jun 24, 2024

Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I pushed a property test. It passes!

@dcherian
Copy link
Contributor

dcherian commented Jun 25, 2024

Oops, the property test did find something. We can skip these in the test if you want though, they seem pretty edge-casey

first = {'': array([False, False])}, second = {'': array([False, False])}

    def equivalent(first: T, second: T) -> bool:
        """Compare two objects for equivalence (identity or equality), using
        array_equiv if either object is an ndarray. If both objects are lists,
        equivalent is sequentially called on all the elements.
        """
        # TODO: refactor to avoid circular import
        from xarray.core import duck_array_ops

        if first is second:
            return True
        if isinstance(first, np.ndarray) or isinstance(second, np.ndarray):
            return duck_array_ops.array_equiv(first, second)
        if isinstance(first, list) or isinstance(second, list):
            return list_equiv(first, second)
>       return (first == second) or (pd.isnull(first) and pd.isnull(second))
E       ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

@keewis
Copy link
Collaborator Author

keewis commented Jun 25, 2024

true. To fix that, we'd have to support recursive attrs, which feels like a bit too much effort. If anything, I'd raise a ValueError stating that assert_* don't support those.

@dcherian dcherian enabled auto-merge (squash) June 30, 2024 14:09
@dcherian dcherian disabled auto-merge June 30, 2024 16:46
@dcherian dcherian merged commit 3deee7b into pydata:main Jun 30, 2024
27 of 28 checks passed
@keewis keewis deleted the attr-comparison branch July 10, 2024 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

assert_identical fails to generate diff when an array is an attribute value
2 participants