Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues with infered_from and add checks for infered_from and depends_on that are strings #1453

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
ed9d022
Validate that infered_from adn depends_on are not strings but sequences
jenshnielsen Jan 23, 2019
234843a
Correct upgrade path
jenshnielsen Jan 23, 2019
c5108f5
Mypy fix
jenshnielsen Jan 23, 2019
3d230ae
Add another db to version 3 generation
jenshnielsen Jan 25, 2019
37d89f0
Add logic to generate v3 db from upgraded v2
jenshnielsen Jan 26, 2019
e382dff
Actually test all parameters in db upgrade
jenshnielsen Jan 25, 2019
a18eab6
correct v3 hash
jenshnielsen Jan 26, 2019
90132ba
add test that performing v3 to v4
jenshnielsen Jan 26, 2019
0ee0463
Add stupid version of v3-v4 upgrade
jenshnielsen Jan 26, 2019
f68c853
adapt tests to new latest version
jenshnielsen Jan 26, 2019
c719e85
Use test generators from other repo
jenshnielsen Feb 4, 2019
70511e2
Correct path
jenshnielsen Feb 4, 2019
3fa7e6c
Update azure build
jenshnielsen Feb 4, 2019
4abcd6e
legacy generation has been moved to it's own repo
jenshnielsen Feb 4, 2019
1c1ef1c
update tests to reflect that there are now 2 inferred params
jenshnielsen Feb 4, 2019
bac7d43
Handle empty inferred_from correctly
jenshnielsen Feb 4, 2019
a52d6ec
Add more asserts to tests
jenshnielsen Feb 4, 2019
67545d1
simplify upgrade logic
jenshnielsen Feb 4, 2019
6ea4cee
test param that is neither dep/dependent
jenshnielsen Feb 4, 2019
3fe5858
test that this is wrong before upgrade
jenshnielsen Feb 4, 2019
fefc025
also handle inferred_from when not dep/dependent
jenshnielsen Feb 4, 2019
64af8e8
simplifty upgrade logic
jenshnielsen Feb 4, 2019
959a79f
docstring
jenshnielsen Feb 4, 2019
62e4160
add a direct v3->v4 upgrade test
jenshnielsen Feb 4, 2019
82aeb96
More clear error message
jenshnielsen Feb 4, 2019
5e317fa
Add tests for invalid paramspecs
jenshnielsen Feb 4, 2019
fbea6cc
Merge branch 'master' into fix/validate_paramspec_input_infered
jenshnielsen Feb 4, 2019
24f3fe9
Document the errors fixed
jenshnielsen Feb 5, 2019
863de65
Update test docstrings
jenshnielsen Feb 6, 2019
6824df5
Correct docstring
jenshnielsen Feb 6, 2019
778de4d
More consistent variable names
jenshnielsen Feb 6, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,14 @@ script:
- |
pip uninstall -y qcodes
pip install -e .
cd qcodes/tests/dataset/legacy_DB_generation
cd ..
git clone https://github.com/QCoDeS/qcodes_generate_test_db.git
cd qcodes_generate_test_db
python generate_version_0.py
python generate_version_1.py
python generate_version_2.py
python generate_version_3.py
cd ../../../..
cd $TRAVIS_BUILD_DIR
pip uninstall -y qcodes
pip install .
- cd qcodes
Expand Down
12 changes: 7 additions & 5 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@ jobs:
mypy qcodes
displayName: "mypy"
- script: |
cd qcodes
python tests\dataset\legacy_DB_generation\generate_version_0.py
python tests\dataset\legacy_DB_generation\generate_version_1.py
python tests\dataset\legacy_DB_generation\generate_version_2.py
python tests\dataset\legacy_DB_generation\generate_version_3.py
cd ..
git clone https://github.com/QCoDeS/qcodes_generate_test_db.git
cd qcodes_generate_test_db
python generate_version_0.py
python generate_version_1.py
python generate_version_2.py
python generate_version_3.py
displayName: "Generate db fixtures"
- script: |
cd qcodes
Expand Down
9 changes: 9 additions & 0 deletions qcodes/dataset/param_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,19 @@ def __init__(self, name: str,
inferred_from = [] if inferred_from is None else inferred_from
depends_on = [] if depends_on is None else depends_on

if isinstance(inferred_from, str):
raise ValueError(f"ParamSpec {self.name} got "
f"string {inferred_from} as inferred_from. "
f"It needs a "
f"Sequence of ParamSpecs or strings")
self._inferred_from.extend(
p.name if isinstance(p, ParamSpec) else p
for p in inferred_from)

if isinstance(depends_on, str):
raise ValueError(f"ParamSpec {self.name} got "
f"string {depends_on} as depends_on. It needs a "
f"Sequence of ParamSpecs or strings")
self._depends_on.extend(
p.name if isinstance(p, ParamSpec) else p
for p in depends_on)
Expand Down
108 changes: 84 additions & 24 deletions qcodes/dataset/sqlite_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ def _2to3_get_paramspecs(conn: ConnectionPlus,
# depend, then the dependent ParamSpecs and finally the rest

for layout_id in list(indeps) + list(deps) + list(the_rest):
(name, label, unit, inferred_from) = layouts[layout_id]
(name, label, unit, inferred_from_str) = layouts[layout_id]
# get the data type
sql = f'PRAGMA TABLE_INFO("{result_table_name}")'
c = transaction(conn, sql)
Expand All @@ -615,34 +615,23 @@ def _2to3_get_paramspecs(conn: ConnectionPlus,
paramtype = row['type']
break

# first possibility: another parameter depends on this parameter
if layout_id in indeps:
jenshnielsen marked this conversation as resolved.
Show resolved Hide resolved
paramspec = ParamSpec(name=name, paramtype=paramtype,
label=label, unit=unit,
inferred_from=inferred_from)
paramspecs[layout_id] = paramspec

# second possibility: this parameter depends on another parameter
elif layout_id in deps:
inferred_from: List[str] = []
depends_on: List[str] = []
jenshnielsen marked this conversation as resolved.
Show resolved Hide resolved

# this parameter depends on another parameter
if layout_id in deps:
setpoints = dependencies[layout_id]
depends_on = [paramspecs[idp].name for idp in setpoints]

paramspec = ParamSpec(name=name,
paramtype=paramtype,
label=label, unit=unit,
depends_on=depends_on,
inferred_from=inferred_from)
paramspecs[layout_id] = paramspec
if inferred_from_str != '':
inferred_from = inferred_from_str.split(', ')

# third possibility: no dependencies
else:
paramspec = ParamSpec(name=name,
paramtype=paramtype,
label=label, unit=unit,
depends_on=[],
inferred_from=[])
paramspecs[layout_id] = paramspec
paramspec = ParamSpec(name=name,
paramtype=paramtype,
label=label, unit=unit,
depends_on=depends_on,
inferred_from=inferred_from)
paramspecs[layout_id] = paramspec

return paramspecs

Expand Down Expand Up @@ -720,6 +709,77 @@ def perform_db_upgrade_2_to_3(conn: ConnectionPlus) -> None:
log.debug(f"Upgrade in transition, run number {run_id}: OK")


@upgrader
def perform_db_upgrade_3_to_4(conn: ConnectionPlus) -> None:
"""
Perform the upgrade from version 3 to version 4. This really
repeats the version 3 upgrade as it originally had two bugs in
jenshnielsen marked this conversation as resolved.
Show resolved Hide resolved
the inferred annotation. inferred_from was passed incorrectly
resulting in the parameter being marked inferred_from for each char
in the inferred_from variable and inferred_from was not handled
correctly for parameters that were neither dependencies nor dependent on
other parameters. Both have since been fixed so rerun the upgrade.
"""

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

# If one run fails, we want the whole upgrade to roll back, hence the
# entire upgrade is one atomic transaction

with atomic(conn) as conn:

result_tables = _2to3_get_result_tables(conn)
layout_ids_all = _2to3_get_layout_ids(conn)
indeps_all = _2to3_get_indeps(conn)
deps_all = _2to3_get_deps(conn)
layouts = _2to3_get_layouts(conn)
dependencies = _2to3_get_dependencies(conn)

pbar = tqdm(range(1, no_of_runs+1))
pbar.set_description("Upgrading database")

for run_id in pbar:

if run_id in layout_ids_all:

result_table_name = result_tables[run_id]
layout_ids = list(layout_ids_all[run_id])
if run_id in indeps_all:
independents = tuple(indeps_all[run_id])
else:
independents = ()
if run_id in deps_all:
dependents = tuple(deps_all[run_id])
else:
dependents = ()

paramspecs = _2to3_get_paramspecs(conn,
layout_ids,
layouts,
dependencies,
dependents,
independents,
result_table_name)

interdeps = InterDependencies(*paramspecs.values())
desc = RunDescriber(interdeps=interdeps)
json_str = desc.to_json()

else:

json_str = RunDescriber(InterDependencies()).to_json()

sql = f"""
UPDATE runs
SET run_description = ?
WHERE run_id == ?
"""
cur = conn.cursor()
cur.execute(sql, (json_str, run_id))
log.debug(f"Upgrade in transition, run number {run_id}: OK")

def _latest_available_version() -> int:
"""Return latest available database schema version"""
return len(_UPGRADE_ACTIONS)
Expand Down
26 changes: 0 additions & 26 deletions qcodes/tests/dataset/legacy_DB_generation/README.md

This file was deleted.

Empty file.
37 changes: 0 additions & 37 deletions qcodes/tests/dataset/legacy_DB_generation/generate_version_0.py

This file was deleted.

36 changes: 0 additions & 36 deletions qcodes/tests/dataset/legacy_DB_generation/generate_version_1.py

This file was deleted.

Loading