quaquel / pyNetLogo

BSD 3-Clause "New" or "Revised" License
81 stars 22 forks source link

Reorder cleanup in repeat_report() #73

Closed steipatr closed 1 year ago

steipatr commented 1 year ago

Change order in which temporary files, handles, and folders are cleaned up in repeat_report(). If accepted, this closes #72 .

EwoutH commented 1 year ago

@steipatr Thanks for opening this PR! If you provide me with instructions how to test / reproduce I can test on Windows.

@quaquel I might have someone also experiencing this error. Could you review this PR and merge it if you approve? Then we could issue a patch release.

EwoutH commented 1 year ago

Thanks for the very quick response, thanks!

Did we (or the CI) test this on a Unix / Linux system?

At some point using rmtree was discussed in #72. What was the consideration for going with this solution?

A potential rmtree looks quite clean:

import shutil

def repeat_report(self, netlogo_reporter, reps, go="go", include_t0=True):
    # ... existing code here ...

    # cleanup temp files and folders
    for fh in fhs:
        os.close(fh)  # free up file handle for re-use

    shutil.rmtree(tempfolder)  # remove folder and its contents

    return results
quaquel commented 1 year ago

I did not test it, nor do I have time atm to test it.

EwoutH commented 1 year ago

Anne confirmed that this solved the problem for her!