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

Temporary directory autoremoval #148

Merged
merged 11 commits into from
Jun 24, 2022
Merged

Temporary directory autoremoval #148

merged 11 commits into from
Jun 24, 2022

Conversation

elinscott
Copy link
Collaborator

@elinscott elinscott commented Jun 23, 2022

This PR adds the option keep_tmpdirs to the workflow block. If set to False, then the workflow, once finished, will delete all the outdirs from the calculations it performed. keep_tmpdirs is True by default.

This PR also changes the default value of from_scratch from False to True, because I think this is better choice for the default.

Unrelatedly, this PR also moves some old test routines from koopmans/ into tests/, which means they are now run as part of the test suite.

@elinscott elinscott added this to the v1.0 release milestone Jun 23, 2022
@pep8-speaks-bot
Copy link
Collaborator

pep8-speaks-bot commented Jun 23, 2022

Hello @elinscott! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-06-23 16:02:38 UTC

@elinscott elinscott linked an issue Jun 23, 2022 that may be closed by this pull request
@elinscott elinscott self-assigned this Jun 23, 2022
@elinscott elinscott added the enhancement New feature or request label Jun 23, 2022
@elinscott elinscott requested a review from nscolonna June 23, 2022 15:20
@codecov-commenter
Copy link

codecov-commenter commented Jun 23, 2022

Codecov Report

Merging #148 (3d17488) into master (cbba3b3) will increase coverage by 6.67%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #148      +/-   ##
==========================================
+ Coverage   70.70%   77.38%   +6.67%     
==========================================
  Files          70       62       -8     
  Lines        5500     5037     -463     
==========================================
+ Hits         3889     3898       +9     
+ Misses       1611     1139     -472     
Impacted Files Coverage Δ
koopmans/qei_to_json.py 92.30% <ø> (ø)
koopmans/settings/_workflow.py 92.59% <ø> (ø)
koopmans/utils/_os.py 76.56% <100.00%> (ø)
koopmans/workflows/_workflow.py 84.19% <100.00%> (+20.72%) ⬆️
koopmans/testing/_check.py
koopmans/testing/__init__.py
koopmans/testing/_utils.py
koopmans/testing/_mock.py
koopmans/testing/_stumble.py
koopmans/testing/_generate_benchmarks.py
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbba3b3...3d17488. Read the comment docs.

Copy link
Collaborator

@nscolonna nscolonna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks just tested in a couple of cases and it's all good for me!

@elinscott elinscott merged commit 17c011c into master Jun 24, 2022
@elinscott elinscott deleted the tmp_dir_autoremoval branch June 24, 2022 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removing tmp directories
4 participants