juami / pytentiostat

python code for the JUAMI potentiostat
BSD 3-Clause "New" or "Revised" License
9 stars 13 forks source link

fix overwrite file issue #128

Closed weiziyuan closed 4 years ago

weiziyuan commented 4 years ago

add warning messages before overwriting files

codecov-io commented 4 years ago

Codecov Report

Merging #128 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #128   +/-   ##
=======================================
  Coverage   72.34%   72.34%           
=======================================
  Files           8        8           
  Lines         282      282           
=======================================
  Hits          204      204           
  Misses         78       78

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 560f248...4b2b131. Read the comment docs.

sbillinge commented 4 years ago

sounds good. I approve merging this, once a new issue has been opened for the desired refactor.....

On Wed, Nov 27, 2019 at 12:58 PM Weizi Yuan notifications@github.com wrote:

@weiziyuan commented on this pull request.

In GUI/code/GUI_CA_exp_creator.py https://github.com/juami/pytentiostat/pull/128#discussion_r351428486:

 filename = os.path.join(data_out_path, data_out_name + "_CA_config.yml")
  • if warning('The file has been saved. Do you want to exit the window?'):
  • filename_parse = filename.split('/')[-1].strip() # this contains the filename without the path,e.g. 0_CA_config.yml

Thanks for the advice! The view right now is largely independent of the model and the controller, but yes, the model and control are not separate. I agree we should fix that, but I'm afraid we don't have enough time before the AMRS. For now, I'm thinking using this for the AMRS and I DO understand it's not as strong, so we can make multiple tests to make sure it won't crash at the ARMS. But in the future, that's definitely something on the schedule. What do you think about this?

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/juami/pytentiostat/pull/128?email_source=notifications&email_token=ABAOWUPTKY7DC5TCI4JM6F3QV2YMDA5CNFSM4JSJ7W6KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCNG2VTA#discussion_r351428486, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAOWUPNKN2EHPHRLZLNXZTQV2YMDANCNFSM4JSJ7W6A .

sbillinge commented 4 years ago

:+1:

On Wed, Nov 27, 2019 at 2:02 PM Weizi Yuan notifications@github.com wrote:

@weiziyuan commented on this pull request.

In GUI/code/GUI_CA_exp_creator.py https://github.com/juami/pytentiostat/pull/128#discussion_r351453385:

 filename = os.path.join(data_out_path, data_out_name + "_CA_config.yml")
  • if warning('The file has been saved. Do you want to exit the window?'):

  • filename_parse = filename.split('/')[-1].strip() # this contains the filename without the path,e.g. 0_CA_config.yml

πŸ‘

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/juami/pytentiostat/pull/128?email_source=notifications&email_token=ABAOWUKCHOB7SWKFHWAOM2TQV273JA5CNFSM4JSJ7W6KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCNHCQGA#discussion_r351453385, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAOWUPQT3TLRUTAD2QN4G3QV273JANCNFSM4JSJ7W6A .