pyControl / code

pyControl GUI and framework code
https://pycontrol.readthedocs.io
GNU General Public License v3.0
23 stars 20 forks source link

New data file format review #83

Closed alustig3 closed 1 year ago

alustig3 commented 1 year ago
ThomasAkam commented 1 year ago

Thanks @alustig3 for the code review.

Regarding whether we should have a drop-down for the file type, my thoughs are:

What do you think?

alustig3 commented 1 year ago

I agree @ThomasAkam about it being best to avoid having to support both formats long term.

Doing a v2.0 release with only .tsv seems reasonable to me, especially if we did a v2.0rc version beforehand to hopefully get some more feedback/testing from others.

ThomasAkam commented 1 year ago

Sounds good, I'm happy with dropping the .txt format for the next v2.0 release. Agree a v2.0rc for testing is a good idea.

Please could you remove the stuff from the settings dialog for setting the file type and then I'll merge this.

alustig3 commented 1 year ago

@ThomasAkam I removed the stuff from the settings dialog

I hardcoded the data logger to use tsv. This was the quickest and easiest path, but maybe you want to also remove the "file_type" argument from data_logger's open_data_file() all together and get rid of the self.file_type from the Data_logger class?

ThomasAkam commented 1 year ago

Thanks, I'll take out the functionallity for generating the .txt files once I have merged this.

I noticed there is some code in dialogs.py defining a Radio_setter class which does not appear to be used anywhere. Should this be removed?

ThomasAkam commented 1 year ago

Thanks Andy!