Closed JhonFlash3008 closed 9 months ago
Thanks for this @JhonFlash3008! As you say the diff is quite large at the moment, I think I'll try to analyse it locally and push some tweaks so that we can see whats actually changed.
My guess is that you saved the files with CRLF (Windows) line endings and forgot to configure git to convert the line endings to Unix style before committing. That means every line in every file is changed.
I see in the commit message that there is also autoformat with Black. That should go in a separate commit...
My guess is that you saved the files with CRLF (Windows) line endings and forgot to configure git to convert the line endings to Unix style before committing. That means every line in every file is changed.
I'd also undone this (for some reason it didn't push before your comments). I think the standalone black + linting fixes PR makes sense (to avoid this exact scenario again), and then another follow-up PR at some point that enforces formatting in the CI.
I've just tried to unpick the merge conflicts after #98, will make sure the code is unchanged during review (and then ideally we can rebase this PR a bit to make clearer what has happened).
Just to be clear @chatcannon, this PR needs a proper review with additional test files etc. before merging, perhaps we could do an intermediate release with the latest updates and then I can spend a bit of time next week on this one?
I prepared a test with a bunch of .mpr and .mpt files from the v11.50. Do you want me to send it ? Do I juste write it in the test_BioLogic.py file and put the test files in the testdata folder ? or is there a different way to porceed ?
I prepared a test with a bunch of .mpr and .mpt files from the v11.50. Do you want me to send it? Do I juste write it in the test_BioLogic.py file and put the test files in the testdata folder ? or is there a different way to porceed ?
Sounds great! Yes, just add the tests to the bottom of the test file with sensible names and we can tidy them up.
This repo uses Git LFS for the files (more efficient way of tracking changes in big files that won't change much). This is normally available in most git distributions; before you commit the additional test files you just need to run git lfs install
, and then, because they already match the lfs patterns in the .gitattributes
file, it should just magically work. Either way it won't break anything if you just commit the files normally, give it a go and we can fix it with follow-up commits!
So I added the tests with multiple BioLogic files as I wanted to know if it worked for various techniques (OCV, PEIS, GEIS, CP, CA, MB, GCPL). I put them in a specific folder, don't know if it's what you wanna do. I created a new assert_MPR_matches_MPT_v2 method in the test_BioLogic, it uses assert_allclose from numpy.testing as I got messages that assert_field_matches is deprecated. Also it manages EIS quality indicators fields that are different in the mpr and mpt at freq > 100kHz. I had to modify the method fieldname_to_dtype in the BioLogic to take into account new columns in the mpt, mainly due to PEIS and GEIS files
It raised a side question for me : when we load an MPT, we modify fields < Ewe > to Ewe and < I > to I. When we load an MPR we don't. The question is, do we want to keep the info that the measurement are averaged ? If yes, then we souldn't rename the columns in the MPTfile, if not then you should rename the columns in the MPRfile to make it easier to postprocess
It raised a side question for me : when we load an MPT, we modify fields < Ewe > to Ewe and < I > to I. When we load an MPR we don't. The question is, do we want to keep the info that the measurement are averaged ? If yes, then we souldn't rename the columns in the MPTfile, if not then you should rename the columns in the MPRfile to make it easier to postprocess
I think that dates back to me not understanding what the < value >
meant and assuming it was the same as value
. It should probably be fixed i.e. do not rename the columns in the MPTfile.
Superseded by #97
Few modifications in the VMPdata_dtype_from_colIDs Added new headers VMPmodule_hdr_v2 Modified MPRfile initialization
Also... Lots of style editing due to autoformat with Black -> lots of useless…information !!
Need to include tests