Merge pull request microsoft#1549 from WilliamHPNielsen/fix/add_versi…

…on_4a_bug_fix

Fix/add version 4a bug fix

.travis.yml

@@ -49,6 +49,7 @@ script:
       python generate_version_1.py
       python generate_version_2.py
       python generate_version_3.py
+      python generate_version_4a.py
       cd $TRAVIS_BUILD_DIR
       pip uninstall -y qcodes
       pip install .

azure-pipelines.yml

@@ -41,6 +41,7 @@ jobs:
           python generate_version_1.py
           python generate_version_2.py
           python generate_version_3.py
+          python generate_version_4a.py
         displayName: "Generate db fixtures"
         condition: succeededOrFailed()
       - script: |

qcodes/dataset/database_fix_functions.py

@@ -0,0 +1,114 @@
+"""
+Sometimes it happens that databases are put into inconsistent/corrupt states.
+This module contains functions to remedy known issues.
+"""
+import json
+import logging
+from typing import Dict, Sequence
+
+from tqdm import tqdm
+
+from qcodes.dataset.descriptions import RunDescriber
+from qcodes.dataset.dependencies import InterDependencies
+from qcodes.dataset.sqlite_base import (ConnectionPlus, atomic,
+                                        atomic_transaction,
+                                        get_parameters,
+                                        get_run_description, get_user_version,
+                                        one,
+                                        select_one_where,
+                                        update_run_description)
+
+log = logging.getLogger(__name__)
+
+
+def fix_version_4a_run_description_bug(conn: ConnectionPlus) -> Dict[str, int]:
+    """
+    Fix function to fix a bug where the RunDescriber accidentally wrote itself
+    to string using the (new) InterDependencies_ object instead of the (old)
+    InterDependencies object. After the first run, this function should be
+    idempotent.
+
+
+    Args:
+        conn: the connection to the database
+
+    Returns:
+        A dict with the fix results ('runs_inspected', 'runs_fixed')
+    """
+
+    user_version = get_user_version(conn)
+
+    if not user_version == 4:
+        raise RuntimeError('Database of wrong version. Will not apply fix. '
+                           'Expected version 4, found version {user_version}')
+
+    no_of_runs_query = "SELECT max(run_id) FROM runs"
+    no_of_runs = one(atomic_transaction(conn, no_of_runs_query), 'max(run_id)')
+    no_of_runs = no_of_runs or 0
+
+    with atomic(conn) as conn:
+
+        pbar = tqdm(range(1, no_of_runs+1))
+        pbar.set_description("Fixing database")
+
+        # collect some metrics
+        runs_inspected = 0
+        runs_fixed = 0
+
+        for run_id in pbar:
+
+            desc_str = get_run_description(conn, run_id)
+            desc_ser = json.loads(desc_str)
+            idps_ser = desc_ser['interdependencies']
+
+            if RunDescriber._is_description_old_style(idps_ser):
+                pass
+            else:
+                new_desc = RunDescriber.from_json(desc_str)
+                update_run_description(conn, run_id, new_desc.to_json())
+                runs_fixed += 1
+
+            runs_inspected += 1
+
+    return {'runs_inspected': runs_inspected, 'runs_fixed': runs_fixed}
+
+
+def fix_wrong_run_descriptions(conn: ConnectionPlus,
+                               run_ids: Sequence[int]) -> None:
+    """
+    NB: This is a FIX function. Do not use it unless your database has been
+    diagnosed with the problem that this function fixes.
+
+    Overwrite faulty run_descriptions by using information from the layouts and
+    dependencies tables. If a correct description is found for a run, that
+    run is left untouched.
+
+    Args:
+        conn: The connection to the database
+        run_ids: The runs to (potentially) fix
+    """
+
+    user_version = get_user_version(conn)
+
+    if not user_version == 3:
+        raise RuntimeError('Database of wrong version. Will not apply fix. '
+                           'Expected version 3, found version {user_version}')
+
+
+    log.info('[*] Fixing run descriptions...')
+    for run_id in run_ids:
+        trusted_paramspecs = get_parameters(conn, run_id)
+        trusted_desc = RunDescriber(
+                           interdeps=InterDependencies(*trusted_paramspecs))
+
+        actual_desc_str = select_one_where(conn, "runs",
+                                           "run_description",
+                                           "run_id", run_id)
+
+        if actual_desc_str == trusted_desc.to_json():
+            log.info(f'[+] Run id: {run_id} had an OK description')
+        else:
+            log.info(f'[-] Run id: {run_id} had a broken description. '
+                     f'Description found: {actual_desc_str}')
+            update_run_description(conn, run_id, trusted_desc.to_json())
+            log.info(f'    Run id: {run_id} has been updated.')

qcodes/dataset/descriptions.py

@@ -59,14 +59,28 @@ def deserialize(cls, ser: Dict[str, Any]) -> 'RunDescriber':
 
         idp: Union[InterDependencies, InterDependencies_]
 
-        if 'paramspecs' in ser['interdependencies'].keys():
+        if cls._is_description_old_style(ser['interdependencies']):
             idp = InterDependencies.deserialize(ser['interdependencies'])
         else:
             idp = InterDependencies_.deserialize(ser['interdependencies'])
         rundesc = cls(interdeps=idp)
 
         return rundesc
 
+    @staticmethod
+    def _is_description_old_style(serialized_object: Dict[str, Any]) -> bool:
+        """
+        Returns True if an old style description is encountered
+        """
+
+        # NOTE: we should probably think carefully about versioning; keeping
+        # the runs description in sync with the API (this file)
+
+        if 'paramspecs' in serialized_object.keys():
+            return True
+        else:
+            return False
+
     @staticmethod
     def _ruamel_importer():
         try:

qcodes/dataset/sqlite_base.py

@@ -2706,37 +2706,3 @@ def remove_trigger(conn: ConnectionPlus, trigger_id: str) -> None:
         name: id of the trigger
     """
     transaction(conn, f"DROP TRIGGER IF EXISTS {trigger_id};")
-
-
-def _fix_wrong_run_descriptions(conn: ConnectionPlus,
-                                run_ids: Sequence[int]) -> None:
-    """
-    NB: This is a FIX function. Do not use it unless your database has been
-    diagnosed with the problem that this function fixes.
-
-    Overwrite faulty run_descriptions by using information from the layouts and
-    dependencies tables. If a correct description is found for a run, that
-    run is left untouched.
-
-    Args:
-        conn: The connection to the database
-        run_ids: The runs to (potentially) fix
-    """
-
-    log.info('[*] Fixing run descriptions...')
-    for run_id in run_ids:
-        trusted_paramspecs = get_parameters(conn, run_id)
-        trusted_desc = RunDescriber(
-                           interdeps=InterDependencies(*trusted_paramspecs))
-
-        actual_desc_str = select_one_where(conn, "runs",
-                                           "run_description",
-                                           "run_id", run_id)
-
-        if actual_desc_str == trusted_desc.to_json():
-            log.info(f'[+] Run id: {run_id} had an OK description')
-        else:
-            log.info(f'[-] Run id: {run_id} had a broken description. '
-                     f'Description found: {actual_desc_str}')
-            update_run_description(conn, run_id, trusted_desc.to_json())
-            log.info(f'    Run id: {run_id} has been updated.')

qcodes/tests/dataset/test_fix_functions.py

@@ -0,0 +1,98 @@
+import os
+
+import pytest
+
+from qcodes.dataset.database_fix_functions import (
+    fix_version_4a_run_description_bug, fix_wrong_run_descriptions)
+import qcodes.tests.dataset
+from qcodes.tests.dataset.temporary_databases import temporarily_copied_DB
+from qcodes.dataset.data_set import DataSet
+from qcodes.dataset.dependencies import InterDependencies_
+from qcodes.dataset.descriptions import RunDescriber
+from qcodes.dataset.sqlite_base import get_user_version
+
+fixturepath = os.sep.join(qcodes.tests.dataset.__file__.split(os.sep)[:-1])
+fixturepath = os.path.join(fixturepath, 'fixtures')
+
+
+def test_version_4a_bugfix():
+    v4fixpath = os.path.join(fixturepath, 'db_files', 'version4a')
+
+    dbname_old = os.path.join(v4fixpath, 'some_runs.db')
+
+    if not os.path.exists(dbname_old):
+        pytest.skip("No db-file fixtures found. You can generate test db-files"
+                    " using the scripts in the legacy_DB_generation folder")
+
+    with temporarily_copied_DB(dbname_old, debug=False, version=4) as conn:
+
+        dd = fix_version_4a_run_description_bug(conn)
+
+        assert dd['runs_inspected'] == 10
+        assert dd['runs_fixed'] == 10
+
+        dd = fix_version_4a_run_description_bug(conn)
+
+        assert dd['runs_inspected'] == 10
+        assert dd['runs_fixed'] == 0
+
+
+def test_version_4a_bugfix_raises():
+
+    v3fixpath = os.path.join(fixturepath, 'db_files', 'version3')
+    dbname_old = os.path.join(v3fixpath, 'some_runs_without_run_description.db')
+
+    if not os.path.exists(dbname_old):
+        pytest.skip(
+            "No db-file fixtures found. You can generate test db-files"
+            " using the scripts in the legacy_DB_generation folder")
+
+    with temporarily_copied_DB(dbname_old, debug=False, version=3) as conn:
+        with pytest.raises(RuntimeError):
+            fix_version_4a_run_description_bug(conn)
+
+
+def test_fix_wrong_run_descriptions():
+    v3fixpath = os.path.join(fixturepath, 'db_files', 'version3')
+
+    dbname_old = os.path.join(v3fixpath, 'some_runs_without_run_description.db')
+
+    if not os.path.exists(dbname_old):
+        pytest.skip(
+            "No db-file fixtures found. You can generate test db-files"
+            " using the scripts in the legacy_DB_generation folder")
+
+    with temporarily_copied_DB(dbname_old, debug=False, version=3) as conn:
+
+        assert get_user_version(conn) == 3
+
+        ds1 = DataSet(conn=conn, run_id=1)
+        expected_description = ds1.description
+
+        empty_description = RunDescriber(InterDependencies_())
+
+        fix_wrong_run_descriptions(conn, [1, 2, 3, 4])
+
+        ds2 = DataSet(conn=conn, run_id=2)
+        assert expected_description == ds2.description
+
+        ds3 = DataSet(conn=conn, run_id=3)
+        assert expected_description == ds3.description
+
+        ds4 = DataSet(conn=conn, run_id=4)
+        assert empty_description == ds4.description
+
+
+def test_fix_wrong_run_descriptions_raises():
+
+    v4fixpath = os.path.join(fixturepath, 'db_files', 'version4a')
+
+    dbname_old = os.path.join(v4fixpath, 'some_runs.db')
+
+    if not os.path.exists(dbname_old):
+        pytest.skip("No db-file fixtures found. You can generate test db-files"
+                    " using the scripts in the legacy_DB_generation folder")
+
+    with temporarily_copied_DB(dbname_old, debug=False, version=4) as conn:
+        with pytest.raises(RuntimeError):
+            fix_wrong_run_descriptions(conn, [1])