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.

Sign up for GitHub

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

Jump to bottom

Update ExportCallback #130

Merged
merged 6 commits into from
Jul 11, 2022
Merged

Update ExportCallback #130

merged 6 commits into from
Jul 11, 2022

Conversation

skim0119
Copy link
Collaborator

Note

resolve #127

  • Fix bugs in saving
  • Parameter filename and directory is added for clarity.
  • New directory will be created if it does not exist.
  • Added get_last_saved_path method for path check.
  • Added close and clear instead of __del__ which could be more vulnerable to other exceptions.
  • Added more test cases.

@skim0119 skim0119 added the bug Something isn't working label Jul 11, 2022
@skim0119 skim0119 added this to the Version 0.2.4 milestone Jul 11, 2022
@skim0119 skim0119 self-assigned this Jul 11, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2022 •
edited
Loading

Codecov Report

Merging #130 (9009fa6) into update-0.2.4 (fadc859) will increase coverage by 0.48%.
The diff coverage is 100.00%.

@@               Coverage Diff                @@
##           update-0.2.4     #130      +/-   ##
================================================
+ Coverage         86.50%   86.98%   +0.48%     
================================================
  Files                40       40              
  Lines              2578     2590      +12     
  Branches            346      348       +2     
================================================
+ Hits               2230     2253      +23     
+ Misses              325      319       -6     
+ Partials             23       18       -5
Impacted Files Coverage Δ
elastica/callback_functions.py 97.56% <100.00%> (+16.13%) ⬆️

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 03fafd7...9009fa6. Read the comment docs.

armantekinalp
armantekinalp approved these changes Jul 11, 2022
View reviewed changes
Copy link
Contributor

@armantekinalp armantekinalp left a comment

Choose a reason for hiding this comment

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

LGTM

@skim0119 skim0119 merged commit f278806 into GazzolaLab:update-0.2.4 Jul 11, 2022
@skim0119 skim0119 deleted the wip_export branch July 11, 2022 15:09
@skim0119 skim0119 mentioned this pull request Jul 11, 2022
Closed
mstoelzle
mstoelzle reviewed Jul 14, 2022
View reviewed changes
Copy link
Contributor

@mstoelzle mstoelzle left a comment

Choose a reason for hiding this comment

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

I think there is a potential bug / misleading warning when checking the directory

elastica/callback_functions.py Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mstoelzle mstoelzle mstoelzle left review comments

@armantekinalp armantekinalp armantekinalp approved these changes

Assignees

@skim0119 skim0119

Labels
bug Something isn't working
Projects
None yet
Milestone
Version 0.2.4
Development

Successfully merging this pull request may close these issues.
4 participants
@skim0119 @codecov-commenter @mstoelzle @armantekinalp