brainglobe / brainglobe-atlasapi

A lightweight python module to interact with atlases for systems neuroscience
https://brainglobe.info/documentation/brainglobe-atlasapi/index.html
BSD 3-Clause "New" or "Revised" License
117 stars 24 forks source link

Remove slow test marker #300

Closed adamltyson closed 1 month ago

adamltyson commented 1 month ago

This PR closes #297 by removing the slow pytest marker.

I changed the test a bit, but it's still not ideal, because it changes the default brainglobe config file. It changes it back, but that's not great.

I assumed I could get around this by setting monkeypatch.setenv("BRAINGLOBE_CONFIG_DIR", str(tmp_path)), this however doesn't seem to set the config dir temporarily. I don't know if I'm monkeypatching this incorrectly, or there's a bug in the atlas api itself.

I've created this as a draft PR because I think this should be fixed, but maybe this PR should be merged and then an issue created to improve the test? This PR doesn't introduce the problem.

alessandrofelder commented 1 month ago

I've created this as a draft PR because I think this should be fixed, but maybe this PR should be merged and then an issue created to improve the test? This PR doesn't introduce the problem.

I don't mind either way. A new issue is probably cleaner (but also more overhead). One thing maybe worth trying (that I think would be a good idea across BrainGlobe) is to monkeypatch Path.home() for all tests when not running on CI. It looks like there's a few constant variables that need to be adapted for the patching to work. This way

adamltyson commented 1 month ago

To simplify, I've marked this ready to review and raised https://github.com/brainglobe/brainglobe-atlasapi/issues/302