Fix load attachments for same shared cache/cache

[hagenw]

Closes #314.

This ensures that audb.core.load._cached_versions() does not return duplicated entries, when audb.config.SHARED_CACHE_ROOT and audb.config.CACHE_ROOT point to the same folder.
As audb.core.load._get_attachments_from_cache() is setting a folder lock to all folders given by audb.core.load._cached_versions() at the same time, this can then no longer lead to a folder lock error,
and finally it will no longer result in an error/user warning when running audb.load().

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.0%. Comparing base (069cc04) to head (7b8b2b3).
Report is 23 commits behind head on main.

❗ Current head 7b8b2b3 differs from pull request most recent head acd5d13

Please upload reports for the commit acd5d13 to get more accurate results.

Additional details and impacted files

see 12 files with indirect coverage changes

[ChristianGeng]

Purpose

Fixes duplicate attachment problem when the same caches are used in both env pointing to cache directories.

Review

I will approve these. I have only minor stuff:

the test setup is really long, and the assertions are only very short. Would it make sense to refactor this to make the test more readable? In the pytest idiom using fixtures, or even a classical unittest test class with setup and teardown - or a mix of both. Not for functionality, but for readability. As said this is only an option.

I also wonder whether more can / needs to be asserted. I understand that the sort order when duplicates are dropped (keep="first") can not be so easily tested.
As said I am unsure whether reasonable extended assertions exist at all.

[hagenw]

the test setup is really long, and the assertions are only very short. Would it make sense to refactor this to make the test more readable?

This is very often a problem for tests of audformat and audb, as we need to create a database first to be able to perform the test. In principle, this could be done by a fixture that creates and publishes the database, but the problem is that in most cases the database has to have a particular property for the test.
E.g. here it is important that the database has an attachment, and that at least two versions of it exists.
One could have a general database fixture, that gets several arguments to influence the created databases, but I'm not sure if this will increase the readability.

One solution might be to create a publish_database_with_attachments_and_two_versions() fixture at the top of the test_attachments.py file and use it in the test. Is this how you would envision it, or do you have a better suggestion?

[hagenw]

I understand that the sort order when duplicates are dropped (keep="first") can not be so easily tested.

That's correct, but in our special case of the dependency table, whenever an index is duplicated, the whole row is identical. So the dropping order can have no influence. Hence, it should be fine without having a test for it.

[ChristianGeng]

I understand that the sort order when duplicates are dropped (keep="first") can not be so easily tested.

That's correct, but in our special case of the dependency table, whenever an index is duplicated, the whole row is identical. So the dropping order can have no influence. Hence, it should be fine without having a test for it.

Ok for me. keep="first" is the default anyway. But it doesn't harm to be explicit. Approval has already been given.