dgbowl / yadg

yadg: yet another datagram
https://dgbowl.github.io/yadg
GNU General Public License v3.0
35 stars 12 forks source link

Potential localization problem with parsing *.mpt files #95

Closed JAC28 closed 1 year ago

JAC28 commented 1 year ago

I recognized that importing my own Biologic exports (default settings) results in errors while parsing the example files from binder works like a charm. As initally mentioned in #93 the E_ranges will read as str due to a "," instead of a "." as decimal seperator and therefore l.318 in eclabmpt.py will throw an error. I was able to fix this issue locally by the following "quick-and-dirty" solution:

            E_range_max = float(el.get("E_range_max", "inf").replace(',','.')) if isinstance(el.get("E_range_max", "inf"), str) else el.get("E_range_max", "inf")
            E_range_min = float(el.get("E_range_min","-inf").replace(',','.')) if isinstance(el.get("E_range_min", "inf"), str) else el.get("E_range_min", "inf")

I have not fully understood the yadg- workflow, yet, thus this might break something. Please find attached a sample .mpt file, as well as the corresponding .mps-file. To my best knowledge the "," was not set by purpose anywhere in EC-lab software. TestScript_YADG_II_01_MB_C01.mpt.txt TestScript_YADG_II.mps.txt

PeterKraus commented 1 year ago

@vetschn Do you by any chance remember whether the line

Ewe ctrl range : min = -10.00 V, max = 10.00 V

appears in all mpt exports?

PeterKraus commented 1 year ago

Of course, this does not only affect the decimal separator convention, but also affects the timestamp (MM/DD/YYYY vs DD/MM/YYYY).

JAC28 commented 1 year ago

For further information please find attached the corresponding .mpr file. However, I am not able to parse the file due to a NotImplementedError in line 538 of eclabmpr.py. Not sure whether this corresponds to the problem or is caused by anything else.

TestScript_YADG_II_01_MB_C01.mpr.txt

PeterKraus commented 1 year ago

Yeah, I expected that - the value 154 that you mentioned here is likely the culprit.

vetschn commented 1 year ago

@vetschn Do you by any chance remember whether the line

Ewe ctrl range : min = -10.00 V, max = 10.00 V

appears in all mpt exports?

Cannot say with a 100% certainty but I don't remember ever coming across an mpt file without this line in the header.

PeterKraus commented 1 year ago

For further information please find attached the corresponding .mpr file. However, I am not able to parse the file due to a NotImplementedError in line 538 of eclabmpr.py. Not sure whether this corresponds to the problem or is caused by anything else.

TestScript_YADG_II_01_MB_C01.mpr.txt

@JAC28 Thanks a lot for the file. Quick question, was the file created in December (2022-12-08) or in August (2022-08-12)?

JAC28 commented 1 year ago

@PeterKraus I have created the file just for demonstration purpose in december :)

PeterKraus commented 1 year ago

OK, thanks. Parsing mpr files should work with the current master, I'm trying to get the mpt working as well, but it's trickier than I expected. I don't really feel like hard-coding the , -> . replacement, as there should be a way to do it using locales...

PeterKraus commented 1 year ago

@JAC28 yadg-4.2.2 has now been released and should be available via pip, please test whether the issue still persists.

JAC28 commented 1 year ago

Works like a charm. Thank you for the fix.