Imports incorrect for some tests

[yeelauren]

Imports are not consistent with file directory and return

pyspacer/spacer/tests/test_tasks.py

Lines 28 to 30 in 510e9a6

	from spacer.tests.utils import cn_beta_fixture_location
	from spacer.train_utils import make_random_data, train
	from .decorators import require_test_fixtures

There are also references to coralnet's S3 buckets for this test :

https://github.com/coralnet/pyspacer/blob/510e9a655a824d2bea8ee49f7545070274ad2032/spacer/tests/utils.py#L11C1-L17C1

I think these should be using the local filesystem and the /tests/fixtures directory instead ?

[yeelauren]

Same with test_storage.py and test image 08bfc10v7t.png

[StephenChan]

Ah right, the imports would be more consistent if spacer.tests.utils were .utils, grouping it with the .decorators import. (That's what you're pointing out, right?)

So regarding the test fixtures, yeah, some of them have lived in coralnet S3 buckets while some of them have lived in the Git repo's /tests/fixtures. Recently, I was going to pick one of those options to house all the test fixtures, but ended up being indecisive thus far.

I was about to go with tests/fixtures incorporating Git LFS (to address the fact that Git itself isn't efficient at handling large files), but then learned that GitHub's Git LFS bandwidth limits are rather restrictive, so I hesitated on that.

Right now, I'm thinking the way forward might be consolidating and/or simplifying the test fixtures so that they still have the necessary characteristics we want to test, but are smaller in filesize / file count. Then those test fixtures ideally all get housed in the Git repo without Git LFS. Then, if we at the coralnet team still want to archive additional historically tricky images for our records, we can keep those in our own private bucket.

[yeelauren]

Hey Stephen,
I'm trying to verify that the tests are running as expected so that I know what to troubleshoot as I get going.

Ah right, the imports would be more consistent if spacer.tests.utils were .utils, grouping it with the .decorators import. (That's what you're pointing out, right?)

I think the import needs to reference spacer for the decorator to be :

from spacer.tests.utils import cn_beta_fixture_location
from spacer.train_utils import make_random_data, train
from spacer.tests.decorators import require_test_fixtures

Yeah, I've struggled with LFS in the past. Are we able to use the /fixtures directory to run the test cases or are those files not the correct ones?

If more fixtures are needed maybe those are on S3 and pulled down ? ( I'm not even sure what fixtures are used for at this point so I may be missing context!).

[StephenChan]

Ah, I was thinking this for the imports:

from spacer.train_utils import make_random_data, train
from .decorators import require_test_fixtures
from .utils import cn_beta_fixture_location

I have this habit from coralnet where, if I'm in a subdirectory and importing from another file in that same subdirectory, I use the relative-import syntax (like from .module import name) and put these imports at the bottom. As a way of distinguishing the "most local" imports from the other imports. Though to be honest I'm not sure if this is a common Python practice. Are you getting any errors with the relative-import syntax?

Some test files are in /fixtures, some test files are in S3. There might be at least one duplicate as well. It's just not consistent right now. I guess we have to take a pass through the tests to see what files they ask for, and think about what's the most sensible way to consolidate them.