Extract information from temporary files for unfinished experiments #102

JhonFlash3008 · 2024-02-07T17:05:56Z

When acquisition is still running, the mpr file does not contain the log or loop modules. Instead, these data are stored in a mpl and _LOOP.txt file, respectively and are adeed to the mpr files only when the experiment stops (and the temporary files are deleted).
I added 2 functions to extract the loop_index and the timestamp from these temp files during the MPRfile initialization. They are not called if a loop_module and/or a log_module is already found

I wrote the tests but unfortunately I cannot upload the test files, I receive an error saying I exceeded the LFS quota

@JhonFlash3008

Add support for EC-Lab v11.50 Rebased from echemdata#95 by @JhonFlash3008

…p_index and timestamp from the temporary _LOOP.txt and .mpl files during MPRfile initialization Added unitary tests but cannot upload test files due to LFS quota exceeded

from 498 to 502, had to replace other column ids. This is dangerous, as it also modifies dtype from f8 to f4

- supports files without setting module --> extracted loops and cycles - handles conflict between colIDs for the "stack mode" (VMP3 hardware) - supports old files with a timestamp with variable byte index - added lots of new colIDs - supports data_module version 1 - MPRfile attributes startdate, enddate, timestamp are initialized with None value because it is weird if all MPRfile objects don't have the same attributes

…ules, but only if the given argument of MPRfile is a path. This avoids keeping a file busy if an error occurred during class initialization

chatcannon

I would rather see os.path.foo than path.foo.

chatcannon · 2024-03-02T13:48:30Z

galvani/BioLogic.py

@@ -9,7 +9,7 @@

 import re
 import csv
-from os import SEEK_SET
+from os import SEEK_SET, path


Suggested change

from os import SEEK_SET, path

from os import SEEK_SET

import os.path

chatcannon · 2024-03-02T13:49:12Z

galvani/BioLogic.py

@@ -614,49 +828,39 @@ def __init__(self, file_or_path):
                raise ValueError(
                    "Unrecognised version for data module: %d" % data_module["version"]
                )
+        else:
+            if path.isfile(loop_file):


Suggested change

if path.isfile(loop_file):

if os.path.isfile(loop_file):

chatcannon · 2024-03-02T13:49:26Z

galvani/BioLogic.py

                raise ValueError(
                    "Date mismatch:\n"
                    + "    Start date: %s\n" % self.startdate
                    + "    End date: %s\n" % self.enddate
                    + "    Timestamp: %s\n" % self.timestamp
                )
+        else:
+            if path.isfile(log_file):


Suggested change

if path.isfile(log_file):

if os.path.isfile(log_file):

chatcannon · 2024-03-02T13:53:56Z

galvani/BioLogic.py

+        for key, value in stack_mode_colID_dtype_map.items():
+            dtype_map[key] = value


Suggested change

for key, value in stack_mode_colID_dtype_map.items():

dtype_map[key] = value

dtype_map.update(stack_mode_colID_dtype_map)

chatcannon · 2024-03-02T13:55:21Z

galvani/BioLogic.py

-                "Column ID {cid} after column {prev} "
-                "is unknown".format(cid=colID, prev=type_list[-1][0])
-            )
+            if type_list:


Suggested change

if type_list:

if len(type_list) > 0:

Better to make it explicit what you are checking than relying on implicit boolean conversion.

chatcannon · 2024-03-02T13:57:02Z

galvani/BioLogic.py

+            if isinstance(file_or_path, str):
+                fileobj.close()


This should really be done with a contextmanager but the if ...: close() is better than nothing.

chatcannon · 2024-03-02T13:59:08Z

galvani/BioLogic.py

@@ -522,7 +645,83 @@ def read_VMP_modules(fileobj, read_module_data=True):
            yield hdr_dict
            fileobj.seek(hdr_dict["offset"] + hdr_dict["length"], SEEK_SET)

+def loop_from_file(file: str, encoding: str ="latin1"):


Ah, this is the code that matches the description of the pull request.

It would be nicer if you kept this change and then moved the other changes to separate pull requests with their own descriptions.

chatcannon · 2024-03-02T14:01:48Z

galvani/BioLogic.py


        if maybe_log_module:
            (log_module,) = maybe_log_module
            self.enddate = parse_BioLogic_date(log_module["date"])

-            # There is a timestamp at either 465 or 469 bytes


This comment needs to be updated rather than just removed completely.

chatcannon · 2024-03-02T14:02:59Z

tests/test_BioLogic.py

-            index_inf_100k = np.where(mpr.data["freq/Hz"] < 100000)[0]
-            assert_allclose(
-                mpr.data[index_inf_100k][fieldname],
-                mpt[index_inf_100k][fieldname].astype(mpr.data[fieldname].dtype),
-            )
+            if len(mpr.data) == 1:
+                if mpt["THD Ewe/%"] != -1:
+                    assert_allclose(
+                        mpr.data[fieldname],
+                        mpt[fieldname].astype(mpr.data[fieldname].dtype),
+                    )
+            else:
+                indexes = np.where(mpt["THD Ewe/%"] != -1)[0]
+
+                assert_allclose(
+                    mpr.data[indexes][fieldname],
+                    mpt[indexes][fieldname].astype(mpr.data[fieldname].dtype),
+                )


I don't understand what you are changing here and why.

chatcannon · 2024-03-02T14:06:57Z

tests/test_BioLogic.py

+    if mpr.loop_index is None:
+        raise AssertionError("No loop_index found")
+    elif not len(mpr.loop_index) == 4:
+        raise AssertionError("loop_index is not the right size")
+    elif not (mpr.loop_index == [0, 4, 8, 11]).all():
+        raise AssertionError("loop_index values are wrong")


There should be a way to assert all of this in one statement using the pytest numpy integrations.

Suggested change

if mpr.loop_index is None:

raise AssertionError("No loop_index found")

elif not len(mpr.loop_index) == 4:

raise AssertionError("loop_index is not the right size")

elif not (mpr.loop_index == [0, 4, 8, 11]).all():

raise AssertionError("loop_index values are wrong")

assert mpr.loop_index is not None

assert_array_equal(mpr.loop_index, [0, 4, 8, 11])

chatcannon · 2024-03-02T14:07:35Z

tests/test_BioLogic.py

+    mpr = MPRfile(os.path.join(testdata_dir, "running", "running_OCV.mpr"))
+    if not hasattr(mpr, "timestamp"):
+        raise AssertionError("No timestamp found")
+    elif not mpr.timestamp.timestamp() == 1707299985.908:


With floating point numbers you need to use assert_close() or similar, because they will not always be exactly equal.

chatcannon · 2024-03-02T14:11:10Z

galvani/res2sqlite.py

You changed the exec bit on this file which will stop it working as a command line script. Undo that change.

chatcannon · 2024-03-02T14:14:36Z

tests/test_BioLogic.py

+    if not hasattr(mpr, "timestamp"):
+        raise AssertionError("No timestamp found")
+    elif not mpr.timestamp.timestamp() == 1707299985.908:
+        raise AssertionError("timestamp value is wrong")


Suggested change

if not hasattr(mpr, "timestamp"):

raise AssertionError("No timestamp found")

elif not mpr.timestamp.timestamp() == 1707299985.908:

raise AssertionError("timestamp value is wrong")

assert mpr.timestamp.timestamp() == pytest.approx(1707299985.908)

This might still break if you have effects depending on the system timezone. I need to check this myself locally.

chatcannon and others added 7 commits February 7, 2024 10:00

Merge pull request echemdata#97 from chatcannon/JhonFlash-master

a96bee6

Add support for EC-Lab v11.50 Rebased from echemdata#95 by @JhonFlash3008

Merge branch 'master' of https://github.com/JhonFlash3008/galvani

d4d016c

Added loop_from_file and timestamp_from_file functions to extract loo…

8a9d475

…p_index and timestamp from the temporary _LOOP.txt and .mpl files during MPRfile initialization Added unitary tests but cannot upload test files due to LFS quota exceeded

added columns id from various files

6dfde43

from 498 to 502, had to replace other column ids. This is dangerous, as it also modifies dtype from f8 to f4

closing reader buffer before each error and after having read all mod…

870de16

…ules, but only if the given argument of MPRfile is a path. This avoids keeping a file busy if an error occurred during class initialization

Merge branch 'id_finder'

f6939c7

chatcannon reviewed Mar 2, 2024

View reviewed changes

chatcannon mentioned this pull request Mar 2, 2024

Use contextmanagers to ensure files are closed on errors #110

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract information from temporary files for unfinished experiments #102

Extract information from temporary files for unfinished experiments #102

JhonFlash3008 commented Feb 7, 2024

chatcannon left a comment

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

chatcannon Mar 2, 2024

	from os import SEEK_SET, path
	from os import SEEK_SET
	import os.path

		for key, value in stack_mode_colID_dtype_map.items():
		dtype_map[key] = value

	for key, value in stack_mode_colID_dtype_map.items():
	dtype_map[key] = value
	dtype_map.update(stack_mode_colID_dtype_map)

Extract information from temporary files for unfinished experiments #102

Are you sure you want to change the base?

Extract information from temporary files for unfinished experiments #102

Conversation

JhonFlash3008 commented Feb 7, 2024

chatcannon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment