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

Exits working directory if wrapped function throws an exception. #175

Merged
merged 2 commits into from
Oct 4, 2022
Merged

Exits working directory if wrapped function throws an exception. #175

merged 2 commits into from
Oct 4, 2022

Conversation

jevandezande
Copy link
Contributor

If a function wrapped with @utils.work_in or @utils.work_in_tmp_dir throws an exception that is later caught, the working directory will be in the wrong location.

Note: @utils.work_in may have an unintended side-effect (unaffected by this PR). If an empty directory is passed to @utils.work_in, and nothing is produced in that directory, the directory will be deleted.

@t-young31
Copy link
Member

Hi @jevandezande – thanks a lot for the PR, looks great to me. This behaviour is - unhelpfully - only implemented in

def work_in_zipped_dir(zip_path, chdir=True):

which is definitely the right approach 👍🏼


Could I ask a couple of things: (1) change the base to v1.3.1 branch and (2) add yourself to the contributor list on the README, if you're happy to be there, thanks!

@codecov
Copy link

codecov bot commented Oct 4, 2022

Codecov Report

Merging #175 (5c4dc0b) into v1.3.1 (5166fc1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 5c4dc0b differs from pull request most recent head 266980a. Consider uploading reports for the commit 266980a to get more accurate results

@@           Coverage Diff           @@
##           v1.3.1     #175   +/-   ##
=======================================
  Coverage   96.94%   96.94%           
=======================================
  Files         194      194           
  Lines       19785    19807   +22     
=======================================
+ Hits        19180    19202   +22     
  Misses        605      605           
Flag Coverage Δ
unittests 96.94% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
autode/utils.py 95.38% <100.00%> (+0.03%) ⬆️
tests/test_utils.py 95.70% <100.00%> (+0.60%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@t-young31 t-young31 added this to the v1.3.1 milestone Oct 4, 2022
@jevandezande jevandezande changed the base branch from master to v1.3.1 October 4, 2022 12:22
@t-young31 t-young31 merged commit 4b2d950 into duartegroup:v1.3.1 Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants