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 17 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
5 changes: 3 additions & 2 deletions qcodes/dataset/param_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,16 @@ def __init__(self, name: str,

if isinstance(inferred_from, str):
raise ValueError(f"ParamSpec {self.name} got "
f"string {inferred_from} it needs a "
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} it needs a "
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
Expand Down
40 changes: 13 additions & 27 deletions qcodes/dataset/sqlite_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -615,37 +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
inferred_from_list = inferred_from.split(', ')

paramspec = ParamSpec(name=name, paramtype=paramtype,
label=label, unit=unit,
inferred_from=inferred_from_list)
paramspecs[layout_id] = paramspec

# second possibility: this parameter depends on another parameter
elif layout_id in deps:
inferred_from_list: 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]
inferred_from_list = inferred_from.split(', ')

paramspec = ParamSpec(name=name,
paramtype=paramtype,
label=label, unit=unit,
depends_on=depends_on,
inferred_from=inferred_from_list)
paramspecs[layout_id] = paramspec
if inferred_from != '':
inferred_from_list = inferred_from.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_list)
paramspecs[layout_id] = paramspec

return paramspecs

Expand Down Expand Up @@ -727,7 +713,7 @@ def perform_db_upgrade_2_to_3(conn: ConnectionPlus) -> None:
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 a bug in
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. This has since been fixes so rerun
the upgrade.
"""
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.

146 changes: 0 additions & 146 deletions qcodes/tests/dataset/legacy_DB_generation/generate_version_2.py

This file was deleted.

Loading