Remove cache folder #2927
Remove cache folder #2927
Conversation
… in sortingcomponents/benchmark/tests folder
…ss_direction in preprocessing/tests folder
… tmp_path for test_filter_gaussian in preprocessing/tests folder
…n sortingcomponents/test/ folder
…ngcomponents/tests
…one, replace cache_folder with pytest fixture for test_numpy_extractors in core/tests
…ndtruthstudy in comparison/tests/ folder
… function for test_node_pipeline.py in core/tests/ folder
…ation function for test_npyfoldersnippets.py in core/tests/ folder
…ation function for test_motion.py in preprocessing/tests/ folder
…ate_cache_folder function for test_silence.py in preprocessing/tests/
…he_folder function for test_binaryrecordingextractor.py in core/tests/
…ate_cache_folder function for test_whiten.py in preprocessing/tests/
…he_folder function for test_unitsaggregationsorting.py in core/tests/
# Conflicts: # src/spikeinterface/core/tests/test_core_tools.py # src/spikeinterface/preprocessing/tests/test_motion.py
paulrignanese
force-pushed
the
remove_cache_folder
branch
from
91c15b3 to
e7d55e1
Compare
for more information, see https://pre-commit.ci
…he_folder function for test_time_handling.py in core/tests/
…he_folder function for test_time_handling.py in core/tests/
…he_folder function for test_basesorting.py in core/tests/
…ip.py in preprocessing/tests/
…reation for test_benchmark_peak_localization.py in sortingcomponents/benchmark/tests/ folder, even if here the tests are skipped
…_creation for test_sorting_folder.py in core/tests/ folder
… for test_globals.py in core/tests/ folder
… cache_folder by pytest fixture for test_docker_containers.py in sorters/external/tests (fails possibly because of docker not working on my machine)
…w inherited as self.cache_folder in Kilosort4SorterCommonTestSuite for test_kilosort4.py in sorters/external/tests folder
…eplace cache_folder by pytest fixture for test_singularity_containers.py in sorters/external/tests
…st_singularity_containers_gpu.py in sorters/external/tests
… need to be replaced by pytest fixtures however there are not trivial
alejoe91
added
the
hackathon-24
…for test_deepinterpolation.py in deepinterpolation/tests folder (I reverted those changes because we thought that it was producing a bug but it was unrelated, it is back now - Still 2 pytest fixtures TODO : test_launcher.py and test_runsorter.py) Last commit for the Hackathon 2024
|
Hey @paulrignanese thanks a lot for this, that's a lot of work! I agree that breaking this up into smaller PRs is a good idea, and will help with fixing some of the conflicts. I have rights on your fork so if it is convenient for you I'm happy to cherry-pick your commits onto new branches on your fork and fix any merge conflicts, then you can open new PRs from these branches. I can let you know the details of this process. However if this will mess with your workflow or you'd like to go ahead with this yourself of course that's no problem! |
|
I agree that this is huge, but it might make sense to fix this since it only involves tests. If @JoeZiminski and @h-mayorquin think that breaking up into smaller pieces, then let's go for it. In that case, we might need some of @JoeZiminski 's git magic! @paulrignanese what do you think? |
|
Sure I'm happy with that ! Let me know
Le mar. 4 juin 2024 à 12:30, Alessio Buccino ***@***.***> a
écrit :
… I agree that this is huge, but it might make sense to fix this since it
only involves tests. If @JoeZiminski <https://github.com/JoeZiminski> and
@h-mayorquin <https://github.com/h-mayorquin> think that breaking up into
smaller pieces, then let's go for it. In that case, we might need some of
@JoeZiminski <https://github.com/JoeZiminski> 's git magic! @paulrignanese
<https://github.com/paulrignanese> what do you think?
—
Reply to this email directly, view it on GitHub
<#2927 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AXS4IWQBKYBTPPWT7PCBFWDZFWQOTAVCNFSM6AAAAABIPE2OUWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBXGMYDAMJWGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Great, thanks Paul! @alejoe91 yes I'm also a little conflicted as if the tests are passing then it should be okay. On balance although it will be more work to break up I think because of the number of conflicts it will be safer to break it up to avoid accidentally reverting any recent merges. I think updating the fork's main main branch to the most recent version, branching off that and then cherry-picking commits will avoid these merge conflicts or make them easier to resolve. I also wanted to go through the test suite anyways to improve my understanding so this seems like a good chance to do it! If everyone is happy I will:
|
|
Hi Paul. |
|
Ok! |
|
Awesome, cheers @alejoe91 @samuelgarcia! |
|
Hey @alejoe91 just to check I don't need to do anything on this PR anymore right? |
|
Thanks @JoeZiminski No I think this is ok, will give it a final read today ;) |
conftest.py
Outdated
|
|
||
| for mark_name in mark_names: | ||
| (pytest.global_test_folder / mark_name).mkdir() | ||
| @pytest.fixture |
There was a problem hiding this comment.
@paulrignanese @JoeZiminski moving the fixture here allows us to remove it from everywhere else, since it's available to the entire pytest session ;)
|
Ok, should be done now :) |
|
Awesome cheers, and thanks a lot @paulrignanese !! |
removed the setting of the cache folder as global variable in test_phase_shift in /preprocessing/tests, to see if it survives the CI tests