Replace tmpdir with tmp_path

[cbrnr]

Fixes #9662.

[cbrnr]

This has not been super fun (simply replacing tmpdir with tmp_path didn't cut it), but now it is almost working except for the following two issues (@larsoner @drammock?):

• Windows is crashing here ("Windows fatal exception: access violation"), any ideas? Edit: this should be fixed by ignore np.MachAr deprecation warning [skip circle] #9930.
• There's one failing test that I haven't fixed yet, because when saving an already loaded Raw object, it now raises a FileExistsError in https://github.com/mne-tools/mne-python/blob/main/mne/utils/check.py#L167, but test_save expects a different error (https://github.com/cbrnr/mne-python/blob/243da7562012baf4622ae9f8752557eb37709e72/mne/io/fiff/tests/test_raw_fiff.py#L1392) that used to be raised here. How would you like me to proceed?

[larsoner]

There's one failing test that I haven't fixed yet, because when saving an already loaded Raw object, it now raises a FileExistsError

This to me suggests that there is some problem with the first check that's supposed to see "if data are not preloaded and we're trying to save to self._filenames[0], raise a ValueError", and instead you hit the second / subsequent check that is "if the filename exists and overwrite is False (default), then raise a FileExistsError". I would look into why using this new fixture has changed the behavior of the first check

[cbrnr]

@larsoner the issue is that fname = _check_fname(fname) raises a FileExistsError: Destination file exists. Please use option "overwrite=True" to force overwriting. even before the other exception (if not self.preload and fname in self._filenames:) can be raised. I could of course set overwrite=True, but I guess this defeats the purpose of the other check. Any suggestions on how to proceed?

[larsoner]

I could of course set overwrite=True, but I guess this defeats the purpose of the other check.

I would use overwrite=True because you don't want/need that function to do an overwrite check, because you're doing a more targeted one later

[cbrnr]

This is now ready for review. Maybe worth a changelog entry too, but we can do this as the final step.

[agramfort]

@cbrnr can we now remove tmpdir from our fixtures? Did you already take care of this? 🙏 for the cleanup

[cbrnr]

What do you mean by remove? I replaced tmpdir with tmp_path everywhere, is there some config file I need to update?

[sappelhoff]

And if you're in the flow of it @cbrnr perhaps you wanna do a quick pass on mne-bids as well, replacing the tmpdir -> tmp_path there? 😏

But I see your comment:

This has not been super fun

so if you've had enough for now, we'll just do it some other time when one of us has time and energy

[agramfort]

indeed tmpdir is a pytest builtin fixture. git grep still says: mne/gui/tests/test_coreg_gui.py:def test_coreg_gui_pyvista_file_support(inst_path, tmpdir, mne/gui/tests/test_coreg_gui.py: tempdir = str(tmpdir) mne/gui/tests/test_coreg_gui.py:def test_coreg_gui_pyvista(tmpdir, renderer_interactive_pyvistaqt): mne/gui/tests/test_coreg_gui.py: tempdir = str(tmpdir) is it expected to keep it there?

…

[cbrnr]

@agramfort thanks for double-checking, I missed those. Now there are no tmpdir fixtures anymore.

@sappelhoff I can take a look, mne-bids is much smaller and therefore this should be much faster.

[cbrnr]

@sappelhoff mne-tools/mne-bids#896

[cbrnr]

Any idea what's going on with CircleCI now? https://app.circleci.com/pipelines/github/mne-tools/mne-python/11173/workflows/849f4079-3350-4539-be79-8cdff6404ee1/jobs/37508/parallel-runs/0/steps/0-103

[cbrnr]

@larsoner now in addition to CircleCI most (all?) Azure jobs are failing because of an issue with OpenGL install?

[larsoner]

#9943

[cbrnr]

Please ping me once that is merged.

[cbrnr]

(This might be something where merge queues could be useful – we should definitely try them when they are available.)

[larsoner]

Please ping me once that is merged.

If you go to the PR and click the "subscribe" you'll get notified automatically

[cbrnr]

@GuillaumeFavelier do you know why coreg._lock_fids is not False? https://github.com/mne-tools/mne-python/runs/4081446793?check_suite_focus=true#step:13:3462

[drammock]

Suggested change

	pytest.raises(ValueError, stc.save, tmp_path / 'tmp', ftype='foo')
	# different errors for (non)vector STCs
	match = (r"(Invalid value for the 'ftype' par)|"
	r"(VectorSourceEstimate objects can only be written ad HDF5)*")
	with pytest.raises(ValueError, match=match):
	stc.save(tmp_path / 'tmp', ftype='foo')

[cbrnr]

Should we go with this more specific error message catching or wait for the fix in #9944?

[larsoner]

I don't think my fix in #9944 will impact this comment -- it looks like this comment just improves/modernizes the pytest.raises usage. So yes feel free to commit this suggestion separately

[larsoner]

LGTM, will merge once CIs come back happy. Thanks @cbrnr

[agramfort]

thx @cbrnr !