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

Add default output file behavior #719

Merged
merged 3 commits into from
Mar 4, 2022
Merged

Conversation

katxiao
Copy link
Contributor

@katxiao katxiao commented Feb 28, 2022

Implements the default output file behavior specified in #693

Write sampling to a file by default. This file is deleted upon sampling completion. Otherwise, the file remains and an error message is shown to the user.

Resolves #693

@katxiao katxiao requested a review from a team as a code owner February 28, 2022 21:28
@katxiao katxiao requested review from pvk-developer and fealho and removed request for a team February 28, 2022 21:28
@katxiao katxiao force-pushed the issue-693-default-output-file branch from f67b6a0 to f2fd393 Compare March 2, 2022 21:20
@codecov-commenter
Copy link

codecov-commenter commented Mar 2, 2022

Codecov Report

Merging #719 (83a11f9) into v0.14.0.dev (9505243) will decrease coverage by 0.04%.
The diff coverage is 75.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           v0.14.0.dev     #719      +/-   ##
===============================================
- Coverage        66.00%   65.95%   -0.05%     
===============================================
  Files               36       36              
  Lines             2665     2685      +20     
===============================================
+ Hits              1759     1771      +12     
- Misses             906      914       +8     
Impacted Files Coverage Δ
sdv/tabular/base.py 78.00% <75.00%> (-1.57%) ⬇️

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 9505243...83a11f9. Read the comment docs.

Copy link
Member

@pvk-developer pvk-developer left a comment

Choose a reason for hiding this comment

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

LGTM! 👍🏻

Copy link
Member

@fealho fealho left a comment

Choose a reason for hiding this comment

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

Do you know what the formatting should be for try/except blocks? For if statements we have an empty line between the if and the else, do we also want to have a new line between the try and the except?

sdv/tabular/base.py Show resolved Hide resolved
@katxiao katxiao force-pushed the issue-693-default-output-file branch from f1aac96 to 83a11f9 Compare March 3, 2022 20:23
@katxiao katxiao merged commit 0a5a4f0 into v0.14.0.dev Mar 4, 2022
@katxiao katxiao deleted the issue-693-default-output-file branch March 4, 2022 00:34
katxiao added a commit that referenced this pull request Mar 4, 2022
* Add default output file behavior

* Add unit tests

* cr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants