Skip to content

Moving setitem to the base class #1044

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 9, 2015
Merged

Moving setitem to the base class #1044

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e46ea59
Moving setitem to the base class
josenavas Apr 7, 2015
8474ca4
addressing @squirrelo's comments
josenavas Apr 7, 2015
380057e
Merge branch 'master' of github.com:qiime/QiiTa into unify-sample-set…
josenavas Apr 8, 2015
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
81 changes: 73 additions & 8 deletions qiita_db/metadata_template/base_metadata_template.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@

from qiita_core.exceptions import IncompetentQiitaDeveloperError
from qiita_db.exceptions import (QiitaDBUnknownIDError,
QiitaDBNotImplementedError)
QiitaDBNotImplementedError,
QiitaDBColumnError)
from qiita_db.base import QiitaObject
from qiita_db.sql_connection import SQLConnectionHandler
from qiita_db.util import (exists_table, get_table_cols,
Expand Down Expand Up @@ -308,17 +309,81 @@ def handler(x):
" in template %d" %
(key, self._id, self._md_template.id))

def __setitem__(self, key, value):
r"""Sets the metadata value for the category `key`
def __setitem__(self, column, value):
r"""Sets the metadata value for the category `column`

Parameters
----------
key : str
The metadata category
value : obj
The new value for the category
column : str
The column to update
value : str
The value to set. This is expected to be a str on the assumption
that psycopg2 will cast as necessary when updating.

Raises
------
QiitaDBColumnError
If the column does not exist in the table
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the ValueError raised when value and database column types are not compatible here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

ValueError
If the value type does not match the one in the DB
"""
raise QiitaDBNotImplementedError()
conn_handler = SQLConnectionHandler()

# try dynamic tables
sql = """SELECT EXISTS (
SELECT column_name
FROM information_schema.columns
WHERE table_name=%s
AND table_schema='qiita'
AND column_name=%s)"""
exists_dynamic = conn_handler.execute_fetchone(
sql, (self._dynamic_table, column))[0]
# try required_sample_info
sql = """SELECT EXISTS (
SELECT column_name
FROM information_schema.columns
WHERE table_name=%s
AND table_schema='qiita'
AND column_name=%s)"""
exists_required = conn_handler.execute_fetchone(
sql, (self._table, column))[0]

if exists_dynamic:
sql = """UPDATE qiita.{0}
SET {1}=%s
WHERE sample_id=%s""".format(self._dynamic_table,
column)
elif exists_required:
# here is not required the type check as the required fields have
# an explicit type check
sql = """UPDATE qiita.{0}
SET {1}=%s
WHERE sample_id=%s""".format(self._table, column)
else:
raise QiitaDBColumnError("Column %s does not exist in %s" %
(column, self._dynamic_table))

try:
conn_handler.execute(sql, (value, self._id))
except Exception as e:
# catching error so we can check if the error is due to different
# column type or something else
column_type = conn_handler.execute_fetchone(
"""SELECT data_type
FROM information_schema.columns
WHERE column_name=%s AND table_schema='qiita'
""", (column,))[0]
value_type = type(value).__name__

if column_type != value_type:
raise ValueError(
'The new value being added to column: "{0}" is "{1}" '
'(type: "{2}"). However, this column in the DB is of '
'type "{3}". Please change the value in your updated '
'template or reprocess your sample template.'.format(
column, value, value_type, column_type))
else:
raise e

def __delitem__(self, key):
r"""Removes the sample with sample id `key` from the database
Expand Down
74 changes: 0 additions & 74 deletions qiita_db/metadata_template/sample_template.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,80 +63,6 @@ def _check_template_class(self, md_template):
if not isinstance(md_template, SampleTemplate):
raise IncompetentQiitaDeveloperError()

def __setitem__(self, column, value):
r"""Sets the metadata value for the category `column`

Parameters
----------
column : str
The column to update
value : str
The value to set. This is expected to be a str on the assumption
that psycopg2 will cast as necessary when updating.

Raises
------
QiitaDBColumnError
If the column does not exist in the table
"""
conn_handler = SQLConnectionHandler()

# try dynamic tables
exists_dynamic = conn_handler.execute_fetchone("""
SELECT EXISTS (
SELECT column_name
FROM information_schema.columns
WHERE table_name='{0}'
AND table_schema='qiita'
AND column_name='{1}')""".format(self._dynamic_table,
column))[0]
# try required_sample_info
exists_required = conn_handler.execute_fetchone("""
SELECT EXISTS (
SELECT column_name
FROM information_schema.columns
WHERE table_name='required_sample_info'
AND table_schema='qiita'
AND column_name='{0}')""".format(column))[0]

if exists_dynamic:
# catching error so we can check if the error is due to different
# column type or something else
try:
conn_handler.execute("""
UPDATE qiita.{0}
SET {1}=%s
WHERE sample_id=%s""".format(self._dynamic_table,
column), (value, self._id))
except Exception as e:
column_type = conn_handler.execute_fetchone("""
SELECT data_type
FROM information_schema.columns
WHERE column_name=%s AND table_schema='qiita'
""", (column,))[0]
value_type = type(value).__name__

if column_type != value_type:
raise ValueError(
'The new value being added to column: "{0}" is "{1}" '
'(type: "{2}"). However, this column in the DB is of '
'type "{3}". Please change the value in your updated '
'template or reprocess your sample template.'.format(
column, value, value_type, column_type))
else:
raise e
elif exists_required:
# here is not required the type check as the required fields have
# an explicit type check
conn_handler.execute("""
UPDATE qiita.required_sample_info
SET {0}=%s
WHERE sample_id=%s
""".format(column), (value, self._id))
else:
raise QiitaDBColumnError("Column %s does not exist in %s" %
(column, self._dynamic_table))


class SampleTemplate(MetadataTemplate):
r"""Represent the SampleTemplate of a study. Provides access to the
Expand Down
12 changes: 9 additions & 3 deletions qiita_db/metadata_template/test/test_prep_template.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,15 @@ def test_get_none(self):
class TestPrepSampleReadWrite(BaseTestPrepSample):
"""Tests the PrepSample class"""
def test_setitem(self):
"""setitem raises an error (currently not allowed)"""
with self.assertRaises(QiitaDBNotImplementedError):
self.tester['barcodesequence'] = 'GTCCGCAAGTTA'
with self.assertRaises(QiitaDBColumnError):
self.tester['column that does not exist'] = 0.3

with self.assertRaises(ValueError):
self.tester['emp_status_id'] = "Error!"

self.assertEqual(self.tester['center_name'], 'ANL')
self.tester['center_name'] = "FOO"
self.assertEqual(self.tester['center_name'], "FOO")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs a test for when a column has the wrong datatype, triggering the ValueError.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great point! When I wrote the test I noticed that there is a "bug" on the code (it is just preventing a useful message, therefore quoted). I'm fixing it right now.


def test_delitem(self):
"""delitem raises an error (currently not allowed)"""
Expand Down
4 changes: 4 additions & 0 deletions qiita_db/metadata_template/test/test_sample_template.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,10 @@ class TestSampleReadWrite(BaseTestSample):
def test_setitem(self):
with self.assertRaises(QiitaDBColumnError):
self.tester['column that does not exist'] = 0.30

with self.assertRaises(ValueError):
self.tester['collection_timestamp'] = "Error!"

self.assertEqual(self.tester['tot_nitro'], 1.41)
self.tester['tot_nitro'] = '1234.5'
self.assertEqual(self.tester['tot_nitro'], 1234.5)
Expand Down