This pull request introduces 1 alert when merging dd4af89 into 5abd8d9 - view on LGTM.com

new alerts:

• 1 for Unused import

@mattwthompson (cc @j-wags ) I don't think the TemporaryDirectory context manager also cd's into the temporary directory, so I think the changes you've made where you now remove the temporary cd will just create files in the cwd.

Also snippets like

        with tempfile.TemporaryDirectory() as tmpdir:
            outfile = 'temp_molecule.' + file_format
            self.to_file(molecule, outfile, file_format)
            file_data = open(outfile).read()

would likely be better to just use a NamedTemporaryFile.

>>> with tempfile.TemporaryDirectory() as tmpdir:
...     Molecule.from_smiles('CCO').to_file('tmp.sdf', file_format='sdf')
... 
>>> os.listdir()
['foo.txt', 'tmp.sdf', 'bar.txt']

That's right. I was fairly surprised the tests passed but that shouldn't have been the criteria anyway. I'll fix this tomorrow.

Ahhh, simon is correct above, just saw this and came to say the same thing -- There are still some places where we need to cd into the temporary directory. I'm adding temporary_cd back in in #471

@j-wags https://github.com/SimonBoothroyd/nonbonded/blob/621151c747925aefeb368d0a2bc99c4305de6dd7/nonbonded/library/utilities/utilities.py#L34 may be helpful - it optionally temporarily cd's into a temporary directory.

Ah, that's really helpful. For now I'm going to revert it to two nested contexts (first make directory, then enter it), since that's more explicit about what's happening. I think this will continue to be a tricky area of the codebase, so "more explicit" will be good here.