Skip to content

Commit

Permalink
Merge #4658
Browse files Browse the repository at this point in the history
4658: DataSet.get_metadata to return value only if the casing of the tag matches the column name in the database r=astafan8 a=astafan8

``DataSet.get_metadata`` method for retrieving a metadata value for a
given tag is now case-sensitive with respect to the tag.
For example, if metadata was added with ``dataset.add_metadata('something', 1)``,
it can only be retrieved by using its exact casing of the tag,
``dataset.get_metadata('something')``, and not e.g.
``dataset.get_metadata('SomeThinG')``. In the previous versions of QCoDeS,
any casing of the tag in ``DataSet.get_metadata`` would work and return the
value stored under that tag. Note that this change brings consistency
with how getting metadata via the ``dataset.metadata`` works:
``dataset.metadata['something']`` does return the value, and
``dataset.metadata['SomeThinG']`` does not.

Also adds extra testing for ``one()`` helper function and clarifies docstrings of a few helper functions.

Co-authored-by: Mikhail Astafev <astafan8@gmail.com>
  • Loading branch information
bors[bot] and astafan8 committed Sep 27, 2022
2 parents 28546e3 + 350f15f commit 1da23ac
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 17 deletions.
11 changes: 11 additions & 0 deletions docs/changes/newsfragments/4658.breaking
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
``DataSet.get_metadata`` method for retrieving a metadata value for a
given tag is now case-sensitive with respect to the tag.
For example, if metadata was added with ``dataset.add_metadata('something', 1)``,
it can only be retrieved by using its exact casing of the tag,
``dataset.get_metadata('something')``, and not e.g.
``dataset.get_metadata('SomeThinG')``. In the previous versions of QCoDeS,
any casing of the tag in ``DataSet.get_metadata`` would work and return the
value stored under that tag. Note that this change brings consistency
with how getting metadata via the ``dataset.metadata`` works:
``dataset.metadata['something']`` does return the value, and
``dataset.metadata['SomeThinG']`` does not.
4 changes: 3 additions & 1 deletion qcodes/dataset/sqlite/queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -1789,7 +1789,9 @@ def get_data_by_tag_and_table_name(
# in an atomic that will do a rollback
# this probably just means that the column is not there
# and therefore it contains no data
if str(e.__cause__).startswith("no such column"):
if str(e.__cause__).startswith("no such column") or str(e).startswith(
"no such column"
):
data = None
else:
raise e
Expand Down
50 changes: 40 additions & 10 deletions qcodes/dataset/sqlite/query_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,14 @@ def get_description_map(curr: sqlite3.Cursor) -> dict[str, int]:

def one(curr: sqlite3.Cursor, column: int | str) -> Any:
"""Get the value of one column from one row
Args:
curr: cursor to operate on
column: name of the column
column: name of the column or the index of the desired column in the
result rows that ``cursor.fetchall()`` returns. In case a
column name is being passed, it is important that the casing
of the column name is exactly the one used when the column was
created.
Returns:
the value
Expand All @@ -52,14 +57,36 @@ def one(curr: sqlite3.Cursor, column: int | str) -> Any:
elif len(res) == 0:
raise RuntimeError("Expected one row")
else:
return res[0][
column if isinstance(column, int) else get_description_map(curr)[column]
]
row = res[0]

if isinstance(column, int):
column_index_in_the_row = column
else:
columns_name_to_index_map = get_description_map(curr)
maybe_column_index = columns_name_to_index_map.get(column)
if maybe_column_index is not None:
column_index_in_the_row = maybe_column_index
else:
# Note that the error message starts the same way as an
# sqlite3 error about a column not existing:
# ``no such column: <column name>`` - this is on purpose,
# because if the given column name is not found in the
# description map from the cursor then something is
# definitely wrong, and likely the requested column does not
# exist or the casing of its name is not equal to the casing
# in the database
raise RuntimeError(
f"no such column: {column}. "
f"Valid columns are {tuple(columns_name_to_index_map.keys())}"
)

return row[column_index_in_the_row]


def _need_to_select(curr: sqlite3.Cursor, *columns: str) -> bool:
"""
Return True if the columns' description of the last query doesn't exactly match
Return True if the columns' description of the last query doesn't exactly match,
the order is important
"""
return tuple(c[0] for c in curr.description) != columns

Expand All @@ -78,7 +105,7 @@ def many(curr: sqlite3.Cursor, *columns: str) -> tuple[Any, ...]:
raise RuntimeError("Expected only one row")
elif _need_to_select(curr, *columns):
raise RuntimeError(
"Expected consistent selection: cursor has columns"
"Expected consistent selection: cursor has columns "
f"{tuple(c[0] for c in curr.description)} but expected {columns}"
)
else:
Expand All @@ -98,7 +125,7 @@ def many_many(curr: sqlite3.Cursor, *columns: str) -> list[tuple[Any, ...]]:

if _need_to_select(curr, *columns):
raise RuntimeError(
"Expected consistent selection: cursor has columns"
"Expected consistent selection: cursor has columns "
f"{tuple(c[0] for c in curr.description)} but expected {columns}"
)

Expand All @@ -116,14 +143,17 @@ def select_one_where(
Args:
conn: Connection to the db
table: Table to look for values in
column: Column to return value from
column: Column to return value from, it is important that the casing
of the column name is exactly the one used when the column was
created
where_column: Column to match on
where_value: Value to match in where_column
Returns:
Value found
raises:
RuntimeError if not exactly match is found.
Raises:
RuntimeError if not exactly one match is found.
"""
query = f"""
SELECT {column}
Expand Down
19 changes: 16 additions & 3 deletions qcodes/tests/dataset/test_metadata.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import pytest

from qcodes.tests.common import error_caused_by


def test_get_metadata_from_dataset(dataset):
dataset.add_metadata('something', 123)
Expand All @@ -10,6 +8,21 @@ def test_get_metadata_from_dataset(dataset):


def test_get_nonexisting_metadata(dataset):

data = dataset.get_metadata("something")
assert data is None


def test_get_metadata_lower_upper_case(dataset):
dataset.add_metadata("something", 123)

something_lower = dataset.metadata.get("something", "didnt find lowercase")
assert something_lower == 123

something_upper = dataset.metadata.get("Something", "didnt find uppercase")
assert something_upper == "didnt find uppercase"

get_something_lower = dataset.get_metadata("something")
assert get_something_lower == 123

get_something_upper = dataset.get_metadata("Something")
assert get_something_upper is None
83 changes: 80 additions & 3 deletions qcodes/tests/dataset/test_sqlite_base.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Since all other tests of data_set and measurements will inevitably also
# test the sqlite module, we mainly test exceptions and small helper
# functions here
import re
import time
import unicodedata
from contextlib import contextmanager
Expand All @@ -16,14 +17,15 @@
from qcodes.dataset.descriptions.dependencies import InterDependencies_
from qcodes.dataset.descriptions.param_spec import ParamSpecBase
from qcodes.dataset.descriptions.rundescriber import RunDescriber
from qcodes.dataset.experiment_container import load_or_create_experiment
from qcodes.dataset.guids import generate_guid

# mut: module under test
from qcodes.dataset.sqlite import connection as mut_conn
from qcodes.dataset.sqlite import database as mut_db
from qcodes.dataset.sqlite import queries as mut_queries
from qcodes.dataset.sqlite import query_helpers as mut_help
from qcodes.dataset.sqlite.connection import path_to_dbfile
from qcodes.dataset.sqlite.connection import atomic_transaction, path_to_dbfile
from qcodes.dataset.sqlite.database import get_DB_location
from qcodes.tests.common import error_caused_by
from qcodes.utils import QCoDeSDeprecationWarning
Expand Down Expand Up @@ -68,13 +70,88 @@ def test_path_to_dbfile(tmp_path):
conn.close()


def test_one_raises(experiment):
def test_one_raises_on_no_results(experiment):
conn = experiment.conn

with pytest.raises(RuntimeError):
with pytest.raises(RuntimeError, match="Expected one row"):
mut_queries.one(conn.cursor(), column='Something_you_dont_have')


def test_one_raises_on_more_than_one_result(experiment):
conn = experiment.conn

# create another experiment with so that the query below returns
# MORE THAN ONE experiment id
load_or_create_experiment(experiment.name + "2", experiment.sample_name)

query = f"""
SELECT exp_id
FROM experiments
"""
cur = atomic_transaction(conn, query)

with pytest.raises(RuntimeError, match="Expected only one row"):
mut_queries.one(cur, column="exp_id")


def test_one_raises_on_wrong_column_name(experiment):
conn = experiment.conn

query = f"""
SELECT exp_id
FROM experiments
"""
cur = atomic_transaction(conn, query)

with pytest.raises(RuntimeError, match=re.escape("no such column: eXP_id")):
mut_queries.one(cur, column="eXP_id")


def test_one_raises_on_wrong_column_index(experiment):
conn = experiment.conn

query = f"""
SELECT exp_id
FROM experiments
"""
cur = atomic_transaction(conn, query)

with pytest.raises(IndexError):
mut_queries.one(cur, column=1)


def test_one_works_if_given_column_index(experiment):
# This test relies on the fact that there's only one experiment in the
# given database
conn = experiment.conn

query = f"""
SELECT exp_id
FROM experiments
"""
cur = atomic_transaction(conn, query)

exp_id = mut_queries.one(cur, column=0)

assert exp_id == experiment.exp_id


def test_one_works_if_given_column_name(experiment):
# This test relies on the fact that there's only one experiment in the
# given database
conn = experiment.conn

query = f"""
SELECT exp_id
FROM experiments
"""
cur = atomic_transaction(conn, query)

exp_id = mut_queries.one(cur, column="exp_id")

assert exp_id == experiment.exp_id


def test_atomic_transaction_raises(experiment):
conn = experiment.conn

Expand Down

0 comments on commit 1da23ac

Please sign in to comment.