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

Convert OrderedDicts to dicts to make diff more understandable #13

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Alexander3
Copy link

Some libs like graphene in my case return OrderedDicts istead of normal dicts.

I have assert like this:

    assert result.data == {
        'campaign': {
            'categories': [
                {'name': 'suggested', 'description': 'represent'},
                {'name': 'test category', 'description': 'simply'},
            ]
        }
    }

And it's hard to spot the difference in test results
Zrzut ekranu z 2019-11-25 10-26-19
Pycharm has even "See difference option":
Zrzut ekranu z 2019-11-25 10-26-44

I will write some tests when I'll have more time, for now please tell me what you think about this idea.

@adamchainz
Copy link
Collaborator

I'm not sure this is the responsibility of this library. pytest-icdiff focusses on giving you a better diff experience. It shouldn't be modifying the data.

If want to compare output OrderedDicts to dicts, I think you're better using a helper in your tests so you can write assert normal_dicts(result.data) == { ....

@Alexander3
Copy link
Author

Yes, but using helper in over 1000 tests makes them less readable, and also slower, because it would convert OrderedDicts even when they are equal. I tried to override pytest_assertrepr_compare in my conftest.py (as I understand it's fired only when assert fails), but it's not working because of pytest-icdiff. Looks like hooks added by libs take precedence. It's weird that both hooks got executed, but only output from pytest-icdiff was used, but anyway I think that if equality operator treats dicts and OrderedDicts the same, we can convert them here to give better experience to user. Since they are equal anyway I don't think of it like modification of data.

@hjwp
Copy link
Owner

hjwp commented Nov 26, 2019

this sounds interesting. i'd be pretty happy to just yolo-convert the ordereddict to normaldicts and then show the diff? maybe add a message to say that's what we did...

@hjwp hjwp force-pushed the master branch 3 times, most recently from cbacb25 to 621b15e Compare August 9, 2022 20:27
@hjwp hjwp changed the base branch from master to main December 5, 2023 11:17
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.

3 participants