recommenders-team / recommenders

Best Practices on Recommendation Systems
https://recommenders-team.github.io/recommenders/intro.html
MIT License
18.81k stars 3.07k forks source link

Review test readme and define best practices #2130

Closed miguelgfierro closed 1 month ago

miguelgfierro commented 2 months ago

Description

The reasoning is to make the tests more explicit. For example, if a function returns True, we want to make sure the assert checks for True and not for a truthy value.

Example:

result = True
assert result is True  # Passes because result is exactly True

result = 1
assert result  # Passes because 1 is a truthy value
assert result is True  # Fails because 1 is not the exact True object

Related Issues

References

Checklist:

miguelgfierro commented 2 months ago

Possible solutions:

SimonYansenZhao commented 1 month ago

@miguelgfierro We got new errors: https://github.com/recommenders-team/recommenders/actions/runs/9954661853/job/27500793010

______________________________ test_download_path ______________________________

    def test_download_path():
        # Check that the temporal path is created and deleted
        with download_path() as path:
            assert os.path.isdir(path) is True
        assert os.path.isdir(path) is False

        # Check the behavior when a path is provided
        tmp_dir = TemporaryDirectory()
        with download_path(tmp_dir.name) as path:
            assert os.path.isdir(path) is True
>       assert os.path.isdir(path) is False
E       AssertionError: assert True is False
E        +  where True = <function isdir at 0x14f2671f63b0>('/tmp/tmpfor_zw3k')
E        +    where <function isdir at 0x14f2671f63b0> = <module 'posixpath' from '/azureml-envs/azureml_f7453134e7e1597f40f148b7ddc0ba6e/lib/python3.10/posixpath.py'>.isdir
E        +      where <module 'posixpath' from '/azureml-envs/azureml_f7453134e7e1597f40f148b7ddc0ba6e/lib/python3.10/posixpath.py'> = os.path

tests/unit/recommenders/datasets/test_download_utils.py:79: AssertionError
______________________________ test_merge_rating _______________________________

rating_true =     userID  itemID  rating
0        1       3       3
1        2       1       5
2        2       4       5
3        2... 12       3
14       3      13       2
15       3      14       1
16       1       1       5
17       1       2       4
rating_pred =     userID  itemID  prediction  rating
0        1      12          12       3
1        2      10          14       5
2... 2
15       3      14           5       1
16       1       3          14       5
17       1      10          13       4

    def test_merge_rating(rating_true, rating_pred):
        y_true, y_pred = merge_rating_true_pred(
            rating_true,
            rating_pred,
            col_user=DEFAULT_USER_COL,
            col_item=DEFAULT_ITEM_COL,
            col_rating=DEFAULT_RATING_COL,
            col_prediction=DEFAULT_PREDICTION_COL,
        )
        target_y_true = np.array([3, 3, 5, 5, 3, 3, 2, 1])
        target_y_pred = np.array([14, 12, 7, 8, 13, 6, 11, 5])

        assert y_true.shape == y_pred.shape
>       assert np.all(y_true == target_y_true) is True
E       assert True is True
E        +  where True = <function all at 0x14f262569a20>(0    3\n1    3..., dtype: int64 == array([3, 3, ..., 3, 3, 2, 1])
E        +    where <function all at 0x14f262569a20> = np.all
E           
E           Use -v to get more diff)

tests/unit/recommenders/evaluation/test_python_evaluation.py:120: AssertionError