Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use tempfile for temporary JSON files. #4206

Closed
prady0t opened this issue Jun 23, 2024 · 3 comments · Fixed by #4270
Closed

Use tempfile for temporary JSON files. #4206

prady0t opened this issue Jun 23, 2024 · 3 comments · Fixed by #4270
Assignees
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve

Comments

@prady0t
Copy link
Contributor

prady0t commented Jun 23, 2024

After merging #4125 , we generate a lot of temporary JSON files while running tests. These files are short-lived and are created and deleted with every test passing, although if the tests fail these files have to be deleted manually.

You can see the number of json files being generated when test suit fails many times :

Screenshot 2024-06-18 at 6 27 55 PM

We can use tempfile to generate temporary files to fix this. See : https://docs.python.org/3/library/tempfile.html

@prady0t
Copy link
Contributor Author

prady0t commented Jun 23, 2024

You can assign this to me, I will open a PR soon.

@kratman
Copy link
Contributor

kratman commented Jun 24, 2024

This is true for notebooks as well. They make images and other files, but do not clean them up

@agriyakhetarpal
Copy link
Member

For the notebooks, I think it's better to not use tempfiles so as to not mislead newcomers to PyBaMM and Python, plus users might want to inspect the images after the notebook/cell has been executed

@agriyakhetarpal agriyakhetarpal added difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve labels Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants