[MAINT] avoid creating files during testing: test_infer_effect_maps #3716

Remi-Gau · 2023-04-21T09:45:14Z

Relates to #3525 (comment)
Closes to #3714

Changes proposed in this pull request:

reimplment with InTemporaryDirectory() in test_infer_effect_maps: should lead to a failure on windows
try an alternative with monkeypatch

github-actions · 2023-04-21T09:45:26Z

👋 @Remi-Gau Thanks for creating a PR!

Until this PR is ready for review, you can include the [WIP] tag in its title, or leave it as a github draft.

Please make sure it is compliant with our contributing guidelines. In particular, be sure it checks the boxes listed below.

PR has an interpretable title.
PR links to Github issue with mention Closes #XXXX (see our documentation on PR structure)
Code is PEP8-compliant (see our documentation on coding style)
Changelog or what's new entry in doc/changes/latest.rst (see our documentation on PR structure)

For new features:

There is at least one unit test per new function / class (see our documentation on testing)
The new feature is demoed in at least one relevant example.

For bug fixes:

There is at least one test that would fail under the original bug conditions.

We will review it as quick as possible, feel free to ping us with questions if needed.

codecov · 2023-04-21T10:01:55Z

Codecov Report

Merging #3716 (4481816) into main (b576d9f) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3716      +/-   ##
==========================================
+ Coverage   91.58%   91.60%   +0.02%     
==========================================
  Files         139      139              
  Lines       16596    16596              
  Branches     3243     3071     -172     
==========================================
+ Hits        15199    15203       +4     
+ Misses       1394     1390       -4     
  Partials        3        3

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

ymzayek · 2023-06-12T09:33:56Z

@Remi-Gau did this work with the monkey patch? The failed builds aren't available any more

Remi-Gau · 2023-06-12T10:14:38Z

Can't remember.
Will fix flake8 so get another run.

…uring_test

Remi-Gau · 2023-06-12T11:54:58Z

Current failure

2023-06-12T11:15:07.1733387Z ================================== FAILURES ===================================
2023-06-12T11:15:07.1733682Z ___________________________ test_infer_effect_maps ____________________________
2023-06-12T11:15:07.1734082Z 
2023-06-12T11:15:07.1734538Z path = 'C:\\Users\\VSSADM~1\\AppData\\Local\\Temp\\tmpag0oxz3a'
2023-06-12T11:15:07.1735678Z onerror = <function TemporaryDirectory._rmtree.<locals>.onerror at 0x000001598F7B0820>
2023-06-12T11:15:07.1735868Z 
2023-06-12T11:15:07.1736034Z     def _rmtree_unsafe(path, onerror):
2023-06-12T11:15:07.1736310Z         try:
2023-06-12T11:15:07.1736531Z             with os.scandir(path) as scandir_it:
2023-06-12T11:15:07.1736769Z                 entries = list(scandir_it)
2023-06-12T11:15:07.1736983Z         except OSError:
2023-06-12T11:15:07.1737211Z             onerror(os.scandir, path, sys.exc_info())
2023-06-12T11:15:07.1737432Z             entries = []
2023-06-12T11:15:07.1737634Z         for entry in entries:
2023-06-12T11:15:07.1737844Z             fullname = entry.path
2023-06-12T11:15:07.1738051Z             if _rmtree_isdir(entry):
2023-06-12T11:15:07.1738255Z                 try:
2023-06-12T11:15:07.1738462Z                     if entry.is_symlink():
2023-06-12T11:15:07.1738690Z                         # This can only happen if someone replaces
2023-06-12T11:15:07.1738946Z                         # a directory with a symlink after the call to
2023-06-12T11:15:07.1739193Z                         # os.scandir or entry.is_dir above.
2023-06-12T11:15:07.1739447Z                         raise OSError("Cannot call rmtree on a symbolic link")
2023-06-12T11:15:07.1739684Z                 except OSError:
2023-06-12T11:15:07.1739910Z                     onerror(os.path.islink, fullname, sys.exc_info())
2023-06-12T11:15:07.1740140Z                     continue
2023-06-12T11:15:07.1740357Z                 _rmtree_unsafe(fullname, onerror)
2023-06-12T11:15:07.1740559Z             else:
2023-06-12T11:15:07.1740752Z                 try:
2023-06-12T11:15:07.1740978Z >                   os.unlink(fullname)
2023-06-12T11:15:07.1741523Z E                   PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\VSSADM~1\\AppData\\Local\\Temp\\tmpag0oxz3a\\mask.nii'
2023-06-12T11:15:07.1741796Z 
2023-06-12T11:15:07.1742028Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\shutil.py:616: PermissionError
2023-06-12T11:15:07.1742227Z 
2023-06-12T11:15:07.1742413Z During handling of the above exception, another exception occurred:
2023-06-12T11:15:07.1742583Z 
2023-06-12T11:15:07.1742802Z func = <built-in function unlink>
2023-06-12T11:15:07.1743161Z path = 'C:\\Users\\VSSADM~1\\AppData\\Local\\Temp\\tmpag0oxz3a\\mask.nii'
2023-06-12T11:15:07.1743706Z exc_info = (<class 'PermissionError'>, PermissionError(13, 'The process cannot access the file because it is being used by another process'), <traceback object at 0x000001598F683380>)
2023-06-12T11:15:07.1743970Z 
2023-06-12T11:15:07.1744127Z     def onerror(func, path, exc_info):
2023-06-12T11:15:07.1744364Z         if issubclass(exc_info[0], PermissionError):
2023-06-12T11:15:07.1744597Z             def resetperms(path):
2023-06-12T11:15:07.1744802Z                 try:
2023-06-12T11:15:07.1745009Z                     _os.chflags(path, 0)
2023-06-12T11:15:07.1745217Z                 except AttributeError:
2023-06-12T11:15:07.1745417Z                     pass
2023-06-12T11:15:07.1745616Z                 _os.chmod(path, 0o700)
2023-06-12T11:15:07.1745802Z     
2023-06-12T11:15:07.1745933Z             try:
2023-06-12T11:15:07.1746505Z                 if path != name:
2023-06-12T11:15:07.1747187Z                     resetperms(_os.path.dirname(path))
2023-06-12T11:15:07.1747531Z                 resetperms(path)
2023-06-12T11:15:07.1747819Z     
2023-06-12T11:15:07.1748093Z                 try:
2023-06-12T11:15:07.1748322Z >                   _os.unlink(path)
2023-06-12T11:15:07.1748918Z E                   PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\VSSADM~1\\AppData\\Local\\Temp\\tmpag0oxz3a\\mask.nii'
2023-06-12T11:15:07.1749365Z 
2023-06-12T11:15:07.1749604Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\tempfile.py:802: PermissionError
2023-06-12T11:15:07.1750486Z 
2023-06-12T11:15:07.1750684Z During handling of the above exception, another exception occurred:
2023-06-12T11:15:07.1750857Z 
2023-06-12T11:15:07.1751016Z     def test_infer_effect_maps():
2023-06-12T11:15:07.1751271Z         from nilearn.glm.second_level.second_level import _infer_effect_maps
2023-06-12T11:15:07.1751510Z     
2023-06-12T11:15:07.1751708Z         with InTemporaryDirectory():
2023-06-12T11:15:07.1751943Z             shapes, rk = ((7, 8, 9, 1), (7, 8, 7, 16)), 3
2023-06-12T11:15:07.1752207Z             mask, fmri_data, design_matrices = write_fake_fmri_data_and_design(
2023-06-12T11:15:07.1752476Z                 shapes,
2023-06-12T11:15:07.1752681Z                 rk
2023-06-12T11:15:07.1752878Z             )
2023-06-12T11:15:07.1753075Z             func_img = load(fmri_data[0])
2023-06-12T11:15:07.1753439Z             second_level_input = pd.DataFrame({'map_name': ["a", "b"],
2023-06-12T11:15:07.1753810Z                                                'effects_map_path': [fmri_data[0],
2023-06-12T11:15:07.1754045Z                                                                     "bar"]})
2023-06-12T11:15:07.1754285Z             assert _infer_effect_maps(second_level_input, "a") == [fmri_data[0]]
2023-06-12T11:15:07.1754653Z             with pytest.raises(ValueError, match="File not found: 'bar'"):
2023-06-12T11:15:07.1754921Z                 _infer_effect_maps(second_level_input, "b")
2023-06-12T11:15:07.1755188Z             assert _infer_effect_maps([fmri_data[0]], None) == [fmri_data[0]]
2023-06-12T11:15:07.1755424Z             contrast = np.eye(rk)[1]
2023-06-12T11:15:07.1755670Z             second_level_input = [FirstLevelModel(mask_img=mask)] * 2
2023-06-12T11:15:07.1755930Z             for i, model in enumerate(second_level_input):
2023-06-12T11:15:07.1756163Z                 model.fit(fmri_data[i],
2023-06-12T11:15:07.1756387Z                           design_matrices=design_matrices[i])
2023-06-12T11:15:07.1756653Z             assert len(_infer_effect_maps(second_level_input, contrast)) == 2
2023-06-12T11:15:07.1756941Z             # Delete objects attached to files to avoid WindowsError when deleting
2023-06-12T11:15:07.1757202Z             # temporary directory (in Windows)
2023-06-12T11:15:07.1757437Z >           del mask, fmri_data, func_img, second_level_input
2023-06-12T11:15:07.1757602Z 
2023-06-12T11:15:07.1757977Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\site-packages\nilearn\glm\tests\test_second_level.py:312: 
2023-06-12T11:15:07.1758310Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
2023-06-12T11:15:07.1758612Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\contextlib.py:120: in __exit__
2023-06-12T11:15:07.1758876Z     next(self.gen)
2023-06-12T11:15:07.1759293Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\site-packages\nibabel\tmpdirs.py:84: in InTemporaryDirectory
2023-06-12T11:15:07.1759592Z     yield tmpdir
2023-06-12T11:15:07.1759859Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\tempfile.py:827: in __exit__
2023-06-12T11:15:07.1760126Z     self.cleanup()
2023-06-12T11:15:07.1760393Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\tempfile.py:831: in cleanup
2023-06-12T11:15:07.1760656Z     self._rmtree(self.name)
2023-06-12T11:15:07.1760929Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\tempfile.py:813: in _rmtree
2023-06-12T11:15:07.1761212Z     _shutil.rmtree(name, onerror=onerror)
2023-06-12T11:15:07.1761493Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\shutil.py:740: in rmtree
2023-06-12T11:15:07.1761762Z     return _rmtree_unsafe(path, onerror)
2023-06-12T11:15:07.1762061Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\shutil.py:618: in _rmtree_unsafe
2023-06-12T11:15:07.1762359Z     onerror(os.unlink, fullname, sys.exc_info())
2023-06-12T11:15:07.1762648Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\tempfile.py:805: in onerror
2023-06-12T11:15:07.1762998Z     cls._rmtree(path)
2023-06-12T11:15:07.1763270Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\tempfile.py:813: in _rmtree
2023-06-12T11:15:07.1764021Z     _shutil.rmtree(name, onerror=onerror)
2023-06-12T11:15:07.1764304Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\shutil.py:740: in rmtree
2023-06-12T11:15:07.1764576Z     return _rmtree_unsafe(path, onerror)
2023-06-12T11:15:07.1764874Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\shutil.py:599: in _rmtree_unsafe
2023-06-12T11:15:07.1765169Z     onerror(os.scandir, path, sys.exc_info())
2023-06-12T11:15:07.1765417Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
2023-06-12T11:15:07.1765569Z 
2023-06-12T11:15:07.1766120Z path = 'C:\\Users\\VSSADM~1\\AppData\\Local\\Temp\\tmpag0oxz3a\\mask.nii'
2023-06-12T11:15:07.1766446Z onerror = <function TemporaryDirectory._rmtree.<locals>.onerror at 0x000001598F61FDC0>
2023-06-12T11:15:07.1766640Z 
2023-06-12T11:15:07.1766806Z     def _rmtree_unsafe(path, onerror):
2023-06-12T11:15:07.1767011Z         try:
2023-06-12T11:15:07.1767227Z >           with os.scandir(path) as scandir_it:
2023-06-12T11:15:07.1767681Z E           NotADirectoryError: [WinError 267] The directory name is invalid: 'C:\\Users\\VSSADM~1\\AppData\\Local\\Temp\\tmpag0oxz3a\\mask.nii'
2023-06-12T11:15:07.1767901Z 
2023-06-12T11:15:07.1768123Z c:\hostedtoolcache\windows\python\3.8.10\x64\lib\shutil.py:596: NotADirectoryError

Remi-Gau · 2023-06-12T11:56:37Z

So the monkeypatch works

Will update the tests.

bthirion

LGTM, thx

ymzayek

LGTM thanks @Remi-Gau

Remi-Gau · 2023-06-13T09:10:13Z

thanks for the reviews!

Remi-Gau added 2 commits April 21, 2023 11:38

break test

8235e88

add monkey patch

b49b3a8

Remi-Gau marked this pull request as ready for review June 12, 2023 10:12

Remi-Gau marked this pull request as draft June 12, 2023 10:32

Remi-Gau added 2 commits June 12, 2023 12:33

Merge remote-tracking branch 'upstream/main' into fix-files_created_d…

9466c6d

…uring_test

flake8 fixes

f2ccef0

failing test as the monkeypath version of it works

65dc009

Remi-Gau changed the title ~~[WIP][MAINT] avoid creating files during testing: test_infer_effect_maps~~ [MAINT] avoid creating files during testing: test_infer_effect_maps Jun 12, 2023

Remi-Gau marked this pull request as ready for review June 12, 2023 11:59

Remi-Gau added 2 commits June 12, 2023 16:33

rm 'created during testing in gitignore'

a388229

rm bids_dataset

4481816

bthirion approved these changes Jun 13, 2023

View reviewed changes

ymzayek approved these changes Jun 13, 2023

View reviewed changes

Remi-Gau merged commit 7639219 into nilearn:main Jun 13, 2023
29 checks passed

Remi-Gau deleted the fix-files_created_during_test branch June 13, 2023 09:10

Remi-Gau mentioned this pull request Jun 13, 2023

Make sure tests do not create non temporary files #3714

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MAINT] avoid creating files during testing: test_infer_effect_maps #3716

[MAINT] avoid creating files during testing: test_infer_effect_maps #3716

Remi-Gau commented Apr 21, 2023 •

edited

github-actions bot commented Apr 21, 2023 •

edited by Remi-Gau

codecov bot commented Apr 21, 2023 •

edited

ymzayek commented Jun 12, 2023

Remi-Gau commented Jun 12, 2023

Remi-Gau commented Jun 12, 2023

Remi-Gau commented Jun 12, 2023

bthirion left a comment

ymzayek left a comment

Remi-Gau commented Jun 13, 2023

[MAINT] avoid creating files during testing: test_infer_effect_maps #3716

[MAINT] avoid creating files during testing: test_infer_effect_maps #3716

Conversation

Remi-Gau commented Apr 21, 2023 • edited

github-actions bot commented Apr 21, 2023 • edited by Remi-Gau

codecov bot commented Apr 21, 2023 • edited

Codecov Report

ymzayek commented Jun 12, 2023

Remi-Gau commented Jun 12, 2023

Remi-Gau commented Jun 12, 2023

Remi-Gau commented Jun 12, 2023

bthirion left a comment

Choose a reason for hiding this comment

ymzayek left a comment

Choose a reason for hiding this comment

Remi-Gau commented Jun 13, 2023

Remi-Gau commented Apr 21, 2023 •

edited

github-actions bot commented Apr 21, 2023 •

edited by Remi-Gau

codecov bot commented Apr 21, 2023 •

edited