1
\$\begingroup\$

I am kind of new to python, so I am pretty sure there are way better elegant ways to do this. I have made a function that is able to compare two list of possible json object paths, and detect what elements do not belong to the other list.

In the docstring you will find a better description with examples.

I will also provide the test class with a lot of examples

It will also be appreciated any help on better docstring, since I am not really sure how to describe correctly what this method exactly does

Note: The code works on both Python 3 and 2.

my_utils.py

"""
Utils
"""


def list_diff(list1, list2):
    """Returns a new list with the items in list1 that do not belong to list2"""
    return list(set(list1) - set(list2))


def get_unknown_data_relations(expected_data_relations_paths, real_data_relations_paths):
    """
    Obtain all those fields which are expected to be data_relations but they are not.

    This function does not check afterwards the real_data_relations_paths, if a expected_data_relations_paths
    is expected to have  any field, it will assume it's correct .
    (['pet.field'],['pet']) is correct, but, (['pet2.field'],['pet']) it is not

    :param expected_data_relations_paths: list of field paths expected to be a data_relations
    :param real_data_relations_paths: list of field paths with real data_relations

    .. Examples:
        expected_data_relations_paths = ['pet'], real_data_relations_paths = ['pet']
        Output = []
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        expected_data_relations_paths = ['name'], real_data_relations_paths = ['pet']
        Output = [name]
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        expected_data_relations_paths = ['name.field'], real_data_relations_paths = ['pet']
        Output = [name.field]
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        expected_data_relations_paths = ['pet'], real_data_relations_paths = ['pet', 'pet2']
        Output = []
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        expected_data_relations_paths = ['pet.field'], real_data_relations_paths = ['pet']
        Output = []
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    """
    filtered = []
    for expected in expected_data_relations_paths:
        temporal_expected = expected
        for data_relation in real_data_relations_paths:
            if related(expected, data_relation):
                temporal_expected = data_relation
                break
        filtered.append(temporal_expected)
    return list_diff(filtered, real_data_relations_paths)


def related(path1, path2):
    """Returns True if path1 is related to path2, False otherwise"""
    return list_diff(path2.split('.'), path1.split('.')) == []


Testing file:

"""
You can auto-discover and run all tests with this command:

    $ pytest -c env_vars [-s]

Documentation:

* https://docs.pytest.org/en/latest/
* https://docs.pytest.org/en/latest/fixture.html
"""
from my_utils import get_unknown_data_relations


def test_unit_get_unknown_data_relations_perfect_match():
    """Tests the most simple case with perfect match"""
    assert len(get_unknown_data_relations(['pet'], ['pet'])) == 0


def test_unit_get_unknown_data_relations_missing_element():
    """Tests a missing element"""
    assert get_unknown_data_relations(['unknown_field'], ['pet']) == ['unknown_field']


def test_unit_get_unknown_data_relations_single_expected_multiple_data_relations():
    """Tests single expected belong to multiple data_relations"""
    assert get_unknown_data_relations(['pet'], ['pet', 'pet2']) == []


def test_unit_get_unknown_data_relations_multiple_expected_multiple_data_relations_exact_dimension():
    """Tests multiple expected belong to multiple data_relations"""
    assert get_unknown_data_relations(['pet', 'pet2'], ['pet', 'pet2']) == []


def test_unit_get_unknown_data_relations_multiple_expected_and_unexpected_multiple_data_relations_exact_dimension():
    """Tests multiple with an  unexpected do not belong to multiple data_relations"""
    assert get_unknown_data_relations(['pet', 'pet4'], ['pet', 'pet2']) == ['pet4']


def test_unit_get_unknown_data_relations_multiple_expected_multiple_data_relations_different_dimension():
    """Tests multiple expected belong to multiple data_relations with different dimension"""
    assert get_unknown_data_relations(['pet', 'pet3'], ['pet', 'pet2', 'pet3']) == []



def test_unit_get_unknown_data_relations_empty_expected_empty_data_relations():
    """Tests empty expected belong to empty data_relations"""
    assert get_unknown_data_relations([], []) == []


def test_unit_get_unknown_data_relations_empty_expected_multiple_data_relations():
    """Tests empty expected belong to multiple data_relations"""
    assert get_unknown_data_relations([], ['pet', 'pet2', 'pet3']) == []


def test_unit_get_unknown_data_relations_expected_embedded_single_data_relation():
    """Tests single expected embedded belong to single data_relations"""
    assert get_unknown_data_relations(['pet.name'], ['pet']) == []


def test_unit_get_unknown_data_relations_expected_embedded_multiple_data_relation():
    """Tests single expected embedded belong to multiple data_relations"""
    assert get_unknown_data_relations(['pet.name'], ['pet', 'pet2']) == []


def test_unit_get_unknown_data_relations_multiple_expected_embedded_multiple_data_relation():
    """Tests multiple expected embedded belong to multiple data_relations"""
    assert get_unknown_data_relations(['pet.name', 'pet2.name'], ['pet', 'pet2']) == []


def test_unit_get_unknown_data_relations_unkown_expected_embedded_single_data_relation():
    """Tests single unknown expected embedded do not belong to single data_relations"""
    assert get_unknown_data_relations(['unknown_relation.unknown_field'], ['pet']) == ['unknown_relation.unknown_field']


def test_unit_get_unknown_data_relations_unkown_expected_embedded_multiple_data_relation():
    """Tests single unknown expected embedded do not belong to multiple data_relations"""
    assert get_unknown_data_relations(
        ['unknown_relation.unknown_field'],
        ['pet', 'pet2']
    ) == ['unknown_relation.unknown_field']


def test_unit_get_unknown_data_relations_some_unkown_expected_embedded_single_data_relation():
    """Tests some unknown expected embedded do not belong to single data_relations"""
    assert get_unknown_data_relations(
        ['unknown_relation.unknown_field', 'pet.field'],
        ['pet']
    ) == ['unknown_relation.unknown_field']


def test_unit_get_unknown_data_relations_some_unkown_expected_embedded_multiple_data_relation():
    """Tests some unknown expected embedded do not belong to multiple data_relations"""
    assert get_unknown_data_relations(
        ['unknown_relation.unknown_field', 'pet.field'],
        ['pet', 'pet2']
    ) == ['unknown_relation.unknown_field']

\$\endgroup\$
1
\$\begingroup\$

For me, this is a little bit unclear

def list_diff(list1, list2):
    """Returns a new list with the items in list1 that do not belong to list2"""
    return list(set(list1) - set(list2))

since you don't just return a new list with the items in list1 that are not in list2, but you also remove duplicates from list1 (which maybe is what you want to do?), and you change the order of the list. If you wanted to "return a new list with the items in list1 that are not in list2" (quoting because I'm not sure what you mean by "do not belong to"), you could also write return [digit for digit in list1 if digit not in list2].

And I think I also find the docstring for

def related(path1, path2):
    """Returns True if path1 is related to path2, False otherwise"""
    return list_diff(path2.split('.'), path1.split('.')) == []

a little bit confusing. I would also like it if you type hinted, or if you expanded on your docstring to include argument and return description; you write a little with reST-style in the function above so why not also here? Speaking of this function, by the way, I like that you're comparing against an empty list. It may bring a tiny overhead in creating the list, but it helps the readability a lot.

I think the "main" function (get_unknown_data_relations) also could benefit from a little bit of work on the docstring. It's descriptive, but 6 examples (if you count also the in-line one) seems a bit exaggerated. It could maybe also be turned into something a bit more pythonic, but it's good enough to not struggle with trying to turn it into a list comprehension.


I like that you've written tests. I don't (have to) write docstrings for my tests, so I would skip that since the names tend to describe the tests well enough, but maybe you have a different workflow. I would recommend looking into fixtures if you haven't. You could also have a look at mutation testing, but maybe you already have and maybe all of your tests are already sufficiently safe.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for your comments!! I appreciate them a lot!! :) Didn't you find anyythin wrong in the anidated loops? It is where I struggled more, needed to write break... I don't find it like a clean style code, specially in python where you can do pretty things as the one you described for my list_diff (Which I'm not used to since I come from java, js, and c) \$\endgroup\$ – Mayday Apr 3 at 8:48
  • \$\begingroup\$ I've updated the answer now @Mayday. \$\endgroup\$ – ades Apr 3 at 9:16
  • \$\begingroup\$ I'm sorry to say that doesn't produce the same results @ades, since the "else path" gets executed extra times (that's why I had a break). But it helped me a lot to learn other stuff, like arguments types declarations, and the view of your more "pythonically" way to do things! :) :) \$\endgroup\$ – Mayday Apr 3 at 9:30
  • \$\begingroup\$ Ahh, my bad. Yeah it actually complicates things a little. I think you could use takewhile, but it's honestly fine to just leave it as it is; it's readable and it works. \$\endgroup\$ – ades Apr 3 at 10:04
  • \$\begingroup\$ Great!! Thank you very much!! Edit please the last non-working part, so I will accept the answer :) \$\endgroup\$ – Mayday Apr 3 at 10:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.