Test not using tmpdir fixture

[richardjgowers]

Running the testsuite generates this file:

testsuite/MDAnalysisTests/data/rmsfit_adk_dims.dcd

Probably because a test is not using the tmpdir fixture.

This test should use the tmpdir fixture and not leave a trace

[orbeckst]

Yes, I have always found that one annoying... just didn't have the time to track it down.

[PicoCentauri]

I just checked the corresponding test. The test checks the default file name, which would not be tested properly for an explicit path using the tmpdir feature.

mdanalysis/testsuite/MDAnalysisTests/analysis/test_align.py

Lines 187 to 190 in 25b27e1

	def test_AlignTraj_outfile_default(self, universe, reference):
	reference.trajectory[-1]
	x = align.AlignTraj(universe, reference)
	assert os.path.basename(x.filename) == 'rmsfit_adk_dims.dcd'

The easiest (but dirty) fix would be to remove the file after the test.

[richardjgowers]

@PicoCentauri the problem with deleting after the test is it assumes the test exits cleanly. It looks like it just needs to be ran in a tmpdir

[PicoCentauri]

I tried this first but this won't help since the filename attributes points in the directory where the trajectory is stored:

mdanalysis/package/MDAnalysis/analysis/align.py

Lines 622 to 624 in 8918d42

	if filename is None:
	path, fn = os.path.split(mobile.trajectory.filename)
	filename = os.path.join(path, prefix + fn)

One could change this by just using the actual file name and ignore its path.

[richardjgowers]

huh, that's a pretty weird design. Ok a pytest fixture which does nothing but cleanup would fix this, something like

@pytest.fixture
def nuke_file():
    try:
        yield
    finally:
        os.remove(filename)

[PicoCentauri]

hmm looks good, I could write a simple patch. But maybe it's better to change the function itself? I think ist more convenient if the file is written to the current directory.

[richardjgowers]

@PicoCentauri yeah a patch would be very welcome!

Not sure about changing the behaviour, we might have to assume someone out there is using this weird behaviour as part of a huge workflow and we can't just break their hypothetical work etc...

[orbeckst]

We could deprecate the old behavior and change it for 1.0. writing to a specific path or current dir is more sane than what we currently have.

…

-- Oliver Beckstein email: orbeckst@gmail.com

Am Oct 12, 2018 um 08:39 schrieb Richard Gowers ***@***.***>: @PicoCentauri yeah a patch would be very welcome! Not sure about changing the behaviour, we might have to assume someone out there is using this weird behaviour as part of a huge workflow and we can't just break their hypothetical work etc... — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.