From dee5763980d85e824a025789a297651bade053ad Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Wed, 20 May 2015 21:12:00 -0700 Subject: [PATCH 1/5] Fixing PrepTemplate creation, deletion and study property --- qiita_db/metadata_template/prep_template.py | 32 ++++++++++++--------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/qiita_db/metadata_template/prep_template.py b/qiita_db/metadata_template/prep_template.py index eeee2e047..2bb69de04 100644 --- a/qiita_db/metadata_template/prep_template.py +++ b/qiita_db/metadata_template/prep_template.py @@ -77,16 +77,13 @@ class PrepTemplate(MetadataTemplate): _filepath_table = 'prep_template_filepath' @classmethod - def create(cls, md_template, raw_data, study, data_type, - investigation_type=None): + def create(cls, md_template, study, data_type, investigation_type=None): r"""Creates the metadata template in the database Parameters ---------- md_template : DataFrame The metadata template file contents indexed by samples Ids - raw_data : RawData - The raw_data to which the prep template belongs to. study : Study The study to which the prep template belongs to. data_type : str or int @@ -111,7 +108,7 @@ def create(cls, md_template, raw_data, study, data_type, # Get a connection handler conn_handler = SQLConnectionHandler() - queue_name = "CREATE_PREP_TEMPLATE_%d" % raw_data.id + queue_name = "CREATE_PREP_TEMPLATE_%d" % study.id conn_handler.create_queue(queue_name) # Check if the data_type is the id or the string @@ -134,14 +131,19 @@ def create(cls, md_template, raw_data, study, data_type, # We need the prep_id for multiple calls below, which currently is not # supported by the queue system. Thus, executing this outside the queue prep_id = conn_handler.execute_fetchone( - "INSERT INTO qiita.prep_template (data_type_id, raw_data_id, " - "investigation_type) VALUES (%s, %s, %s) RETURNING " - "prep_template_id", (data_type_id, raw_data.id, - investigation_type))[0] + "INSERT INTO qiita.prep_template " + "(data_type_id, investigation_type) " + "VALUES (%s, %s) RETURNING prep_template_id", + (data_type_id, investigation_type))[0] cls._add_common_creation_steps_to_queue(md_template, prep_id, conn_handler, queue_name) + # Link the prep template with the study + sql = ("INSERT INTO qiita.study_prep_template " + "(study_id, prep_template_id) VALUES (%s, %s)") + conn_handler.add_to_queue(queue_name, sql, (study.id, prep_id)) + try: conn_handler.execute_queue(queue_name) except Exception: @@ -244,6 +246,11 @@ def delete(cls, id_): cls._id_column), (id_,)) + # Remove the row from study_prep_template + conn_handler.execute( + "DELETE FROM qiita.study_prep_template " + "WHERE {0} = %s".format(cls._id_column), (id_,)) + # Remove the row from prep_template conn_handler.execute( "DELETE FROM qiita.prep_template where " @@ -365,10 +372,9 @@ def study_id(self): The ID of the study with which this prep template is associated """ conn = SQLConnectionHandler() - sql = ("SELECT srd.study_id FROM qiita.prep_template pt JOIN " - "qiita.study_raw_data srd ON pt.raw_data_id = srd.raw_data_id " - "WHERE prep_template_id = %d" % self.id) - study_id = conn.execute_fetchone(sql) + sql = ("SELECT study_id FROM qiita.study_prep_template " + "WHERE prep_template_id=%s") + study_id = conn.execute_fetchone(sql, (self.id,)) if study_id: return study_id[0] else: From e66a5b37bb2ebcecdd4da8b8c743bee344c00fe1 Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Wed, 20 May 2015 21:12:12 -0700 Subject: [PATCH 2/5] Fixing tests --- .../test/test_prep_template.py | 124 +++++++++--------- 1 file changed, 63 insertions(+), 61 deletions(-) diff --git a/qiita_db/metadata_template/test/test_prep_template.py b/qiita_db/metadata_template/test/test_prep_template.py index 7abafbba7..28c47374a 100644 --- a/qiita_db/metadata_template/test/test_prep_template.py +++ b/qiita_db/metadata_template/test/test_prep_template.py @@ -351,7 +351,6 @@ def _set_up(self): self.metadata_prefixed = pd.DataFrame.from_dict(metadata_prefixed_dict, orient='index') - self.test_raw_data = RawData(1) self.test_study = Study(1) self.data_type = "18S" self.data_type_id = 2 @@ -773,36 +772,20 @@ class TestPrepTemplateReadWrite(BaseTestPrepTemplate): def setUp(self): self._set_up() - fd, seqs_fp = mkstemp(suffix='_seqs.fastq') - close(fd) - fd, barcodes_fp = mkstemp(suffix='_barcodes.fastq') - close(fd) - filepaths = [(seqs_fp, 1), (barcodes_fp, 2)] - with open(seqs_fp, "w") as f: - f.write("\n") - with open(barcodes_fp, "w") as f: - f.write("\n") - self.new_raw_data = RawData.create(2, [Study(1)], filepaths=filepaths) - - db_test_raw_dir = join(get_db_files_base_dir(), 'raw_data') - db_seqs_fp = join(db_test_raw_dir, "5_%s" % basename(seqs_fp)) - db_barcodes_fp = join(db_test_raw_dir, "5_%s" % basename(barcodes_fp)) - self._clean_up_files = [db_seqs_fp, db_barcodes_fp] + self._clean_up_files = [] def test_create_duplicate_header(self): """Create raises an error when duplicate headers are present""" self.metadata['STR_COLUMN'] = pd.Series(['', '', ''], index=self.metadata.index) with self.assertRaises(QiitaDBDuplicateHeaderError): - PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type) + PrepTemplate.create(self.metadata, self.test_study, self.data_type) def test_create_bad_sample_names(self): # set a horrible list of sample names self.metadata.index = ['o()xxxx[{::::::::>', 'sample.1', 'sample.3'] with self.assertRaises(QiitaDBColumnError): - PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type) + PrepTemplate.create(self.metadata, self.test_study, self.data_type) def test_create_unknown_sample_names(self): # set two real and one fake sample name @@ -812,19 +795,19 @@ def test_create_unknown_sample_names(self): orient='index') # Test error raised and correct error given with self.assertRaises(QiitaDBExecutionError) as err: - PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type) + PrepTemplate.create(self.metadata, self.test_study, self.data_type) self.assertEqual( - str(err.exception), 'Samples found in prep template but not sample' - ' template: 1.NOTREAL') + str(err.exception), + 'Samples found in prep template but not sample template: 1.NOTREAL' + ) def test_create_shorter_prep_template(self): # remove one sample so not all samples in the prep template del self.metadata_dict['SKB7.640196'] self.metadata = pd.DataFrame.from_dict(self.metadata_dict, orient='index') - pt = PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type) + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type) # make sure the two samples were added correctly self.assertEqual(pt.id, 2) @@ -875,8 +858,7 @@ def test_create_error_cleanup(self): exp_id = get_count("qiita.prep_template") + 1 with self.assertRaises(QiitaDBExecutionError): - PrepTemplate.create(metadata, self.new_raw_data, - self.test_study, self.data_type) + PrepTemplate.create(metadata, self.test_study, self.data_type) sql = """SELECT EXISTS( SELECT * FROM qiita.prep_template @@ -893,6 +875,11 @@ def test_create_error_cleanup(self): WHERE prep_template_id=%s)""" self.assertFalse(self.conn_handler.execute_fetchone(sql, (exp_id,))[0]) + sql = """SELECT EXISTS( + SELECT * FROM qiita.study_prep_template + WHERE prep_template_id=%s)""" + self.assertFalse(self.conn_handler.execute_fetchone(sql, (exp_id,))[0]) + self.assertFalse(exists_table("prep_%d" % exp_id, self.conn_handler)) def _common_creation_checks(self, new_id, pt, fp_count): @@ -905,7 +892,13 @@ def _common_creation_checks(self, new_id, pt, fp_count): (new_id,)) # prep_template_id, data_type_id, raw_data_id, preprocessing_status, # investigation_type - self.assertEqual(obs, [[new_id, 2, 5, 'not_preprocessed', None]]) + self.assertEqual(obs, [[new_id, 2, None, 'not_preprocessed', None]]) + + # The prep template has been linked to the study + obs = self.conn_handler.execute_fetchall( + "SELECT * FROM qiita.study_prep_template " + "WHERE prep_template_id=%s", (new_id,)) + self.assertEqual(obs, [[self.test_study.id, new_id]]) # The relevant rows to prep_template_sample have been added. obs = self.conn_handler.execute_fetchall( @@ -991,8 +984,8 @@ def test_create(self): """Creates a new PrepTemplate""" fp_count = get_count('qiita.filepath') new_id = get_count('qiita.prep_template') + 1 - pt = PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type) + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type) self._common_creation_checks(new_id, pt, fp_count) def test_create_already_prefixed_samples(self): @@ -1000,8 +993,8 @@ def test_create_already_prefixed_samples(self): fp_count = get_count('qiita.filepath') new_id = get_count('qiita.prep_template') + 1 pt = npt.assert_warns(QiitaDBWarning, PrepTemplate.create, - self.metadata_prefixed, self.new_raw_data, - self.test_study, self.data_type) + self.metadata_prefixed, self.test_study, + self.data_type) self._common_creation_checks(new_id, pt, fp_count) def test_generate_files(self): @@ -1032,8 +1025,8 @@ def test_create_data_type_id(self): """Creates a new PrepTemplate passing the data_type_id""" fp_count = get_count('qiita.filepath') new_id = get_count('qiita.prep_template') + 1 - pt = PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type_id) + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type_id) self._common_creation_checks(new_id, pt, fp_count) def test_create_warning(self): @@ -1043,8 +1036,7 @@ def test_create_warning(self): new_id = get_count('qiita.prep_template') + 1 del self.metadata['barcode'] pt = npt.assert_warns(QiitaDBWarning, PrepTemplate.create, - self.metadata, self.new_raw_data, - self.test_study, self.data_type) + self.metadata, self.test_study, self.data_type) # The returned object has the correct id self.assertEqual(pt.id, new_id) @@ -1055,7 +1047,13 @@ def test_create_warning(self): (new_id,)) # prep_template_id, data_type_id, raw_data_id, preprocessing_status, # investigation_type - self.assertEqual(obs, [[new_id, 2, 5, 'not_preprocessed', None]]) + self.assertEqual(obs, [[new_id, 2, None, 'not_preprocessed', None]]) + + # The prep template has been linked to the study + obs = self.conn_handler.execute_fetchall( + "SELECT * FROM qiita.study_prep_template " + "WHERE prep_template_id=%s", (new_id,)) + self.assertEqual(obs, [[self.test_study.id, new_id]]) # The relevant rows to prep_template_sample have been added. obs = self.conn_handler.execute_fetchall( @@ -1136,9 +1134,8 @@ def test_create_warning(self): def test_create_investigation_type_error(self): """Create raises an error if the investigation_type does not exists""" with self.assertRaises(QiitaDBColumnError): - PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type_id, - 'Not a term') + PrepTemplate.create(self.metadata, self.test_study, + self.data_type_id, 'Not a term') def test_delete_error(self): """Try to delete a prep template that already has preprocessed data""" @@ -1152,29 +1149,35 @@ def test_delete_unkonwn_id_error(self): def test_delete(self): """Deletes prep template 2""" - pt = PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type_id) + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type_id) PrepTemplate.delete(pt.id) obs = self.conn_handler.execute_fetchall( - "SELECT * FROM qiita.prep_template WHERE prep_template_id=2") + "SELECT * FROM qiita.prep_template WHERE prep_template_id=%s", + (pt.id,)) exp = [] self.assertEqual(obs, exp) + obs = self.conn_handler.execute_fetchall( + "SELECT * FROM qiita.study_prep_template " + "WHERE prep_template_id=%s", (pt.id,)) + obs = self.conn_handler.execute_fetchall( "SELECT * FROM qiita.prep_template_sample " - "WHERE prep_template_id=2") + "WHERE prep_template_id=%s", (pt.id,)) exp = [] self.assertEqual(obs, exp) obs = self.conn_handler.execute_fetchall( - "SELECT * FROM qiita.prep_columns WHERE prep_template_id=2") + "SELECT * FROM qiita.prep_columns WHERE prep_template_id=%s", + (pt.id,)) exp = [] self.assertEqual(obs, exp) with self.assertRaises(QiitaDBExecutionError): self.conn_handler.execute_fetchall( - "SELECT * FROM qiita.prep_2") + "SELECT * FROM qiita.prep_%d" % pt.id) def test_setitem(self): """setitem raises an error (currently not allowed)""" @@ -1191,8 +1194,8 @@ def test_to_file(self): """to file writes a tab delimited file with all the metadata""" fd, fp = mkstemp() close(fd) - pt = PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type) + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type) pt.to_file(fp) self._clean_up_files.append(fp) with open(fp, 'U') as f: @@ -1206,14 +1209,14 @@ def test_preprocessing_status(self): self.assertEqual(pt.preprocessing_status, 'success') # not preprocessed case - pt = PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type_id) + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type_id) self.assertEqual(pt.preprocessing_status, 'not_preprocessed') def test_preprocessing_status_setter(self): """Able to update the preprocessing status""" - pt = PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type_id) + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type_id) self.assertEqual(pt.preprocessing_status, 'not_preprocessed') pt.preprocessing_status = 'preprocessing' self.assertEqual(pt.preprocessing_status, 'preprocessing') @@ -1222,8 +1225,8 @@ def test_preprocessing_status_setter(self): def test_preprocessing_status_setter_failed(self): """Able to update preprocessing_status with a failure message""" - pt = PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type_id) + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type_id) state = 'failed: some error message' self.assertEqual(pt.preprocessing_status, 'not_preprocessed') pt.preprocessing_status = state @@ -1236,8 +1239,8 @@ def test_preprocessing_status_setter_valueerror(self): def test_investigation_type_setter(self): """Able to update the investigation type""" - pt = PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type_id) + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type_id) self.assertEqual(pt.investigation_type, None) pt.investigation_type = "Other" self.assertEqual(pt.investigation_type, 'Other') @@ -1261,8 +1264,8 @@ def test_status(self): # New prep templates have the status to sandbox because there is no # processed data associated with them - pt = PrepTemplate.create(self.metadata, self.new_raw_data, - self.test_study, self.data_type_id) + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type_id) self.assertEqual(pt.status, 'sandbox') def test_update_category(self): @@ -1308,8 +1311,7 @@ def test_check_restrictions(self): del self.metadata['primer'] pt = npt.assert_warns(QiitaDBWarning, PrepTemplate.create, - self.metadata, self.new_raw_data, - self.test_study, self.data_type) + self.metadata, self.test_study, self.data_type) obs = pt.check_restrictions( [PREP_TEMPLATE_COLUMNS['EBI'], From 6b3e23daece60ce91290dd4aea5756280fec0357 Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Thu, 21 May 2015 09:31:04 -0700 Subject: [PATCH 3/5] Add raw data setter and tests, also improving test for the raw data property --- qiita_db/metadata_template/prep_template.py | 25 +++++++++++++++-- .../test/test_prep_template.py | 28 +++++++++++++++---- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/qiita_db/metadata_template/prep_template.py b/qiita_db/metadata_template/prep_template.py index 2bb69de04..4bdd0d348 100644 --- a/qiita_db/metadata_template/prep_template.py +++ b/qiita_db/metadata_template/prep_template.py @@ -279,9 +279,30 @@ def data_type(self, ret_id=False): @property def raw_data(self): conn_handler = SQLConnectionHandler() - return conn_handler.execute_fetchone( + result = conn_handler.execute_fetchone( "SELECT raw_data_id FROM qiita.prep_template " - "WHERE prep_template_id=%s", (self.id,))[0] + "WHERE prep_template_id=%s", (self.id,)) + if result: + return result[0] + return None + + @raw_data.setter + def raw_data(self, raw_data): + conn_handler = SQLConnectionHandler() + sql = """SELECT ( + SELECT raw_data_id + FROM qiita.prep_template + WHERE prep_template_id=%s) + IS NOT NULL""" + exists = conn_handler.execute_fetchone(sql, (self.id,))[0] + if exists: + raise QiitaDBError( + "Prep template %d already has a raw data associated" + % self.id) + sql = """UPDATE qiita.prep_template + SET raw_data_id = %s + WHERE prep_template_id = %s""" + conn_handler.execute(sql, (raw_data.id, self.id)) @property def preprocessed_data(self): diff --git a/qiita_db/metadata_template/test/test_prep_template.py b/qiita_db/metadata_template/test/test_prep_template.py index 28c47374a..e53c866b4 100644 --- a/qiita_db/metadata_template/test/test_prep_template.py +++ b/qiita_db/metadata_template/test/test_prep_template.py @@ -24,7 +24,8 @@ QiitaDBDuplicateHeaderError, QiitaDBExecutionError, QiitaDBColumnError, - QiitaDBWarning) + QiitaDBWarning, + QiitaDBError) from qiita_db.sql_connection import SQLConnectionHandler from qiita_db.study import Study from qiita_db.data import RawData, ProcessedData @@ -535,10 +536,6 @@ def test_data_type_id(self): """data_type returns the int with the data_type_id""" self.assertTrue(self.tester.data_type(ret_id=True), 2) - def test_raw_data(self): - """Returns the raw_data associated with the prep template""" - self.assertEqual(self.tester.raw_data, 1) - def test_preprocessed_data(self): """Returns the preprocessed data list generated from this template""" self.assertEqual(self.tester.preprocessed_data, [1, 2]) @@ -1318,6 +1315,27 @@ def test_check_restrictions(self): PREP_TEMPLATE_COLUMNS_TARGET_GENE['demultiplex']]) self.assertEqual(obs, {'primer'}) + def test_raw_data(self): + """Returns the raw_data associated with the prep template""" + self.assertEqual(self.tester.raw_data, 1) + + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type_id) + self.assertEqual(pt.raw_data, None) + + def test_raw_data_setter_error(self): + rd = RawData(1) + with self.assertRaises(QiitaDBError): + self.tester.raw_data = rd + + def test_raw_data_setter(self): + rd = RawData(1) + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type_id) + self.assertEqual(pt.raw_data, None) + pt.raw_data = rd + self.assertEqual(pt.raw_data, 1) + EXP_PREP_TEMPLATE = ( 'sample_name\tbarcode\tcenter_name\tcenter_project_name\t' From ccdcbc8f92e519819ee7d04988d7d7a725746f09 Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Sun, 24 May 2015 20:50:50 -0500 Subject: [PATCH 4/5] Adding check on prep template deletion: it cannot be removed if a RawData is attached to it. Adding a test for it too --- qiita_db/metadata_template/prep_template.py | 12 ++++++++++++ .../metadata_template/test/test_prep_template.py | 8 ++++++++ 2 files changed, 20 insertions(+) diff --git a/qiita_db/metadata_template/prep_template.py b/qiita_db/metadata_template/prep_template.py index 4bdd0d348..45ab2c87e 100644 --- a/qiita_db/metadata_template/prep_template.py +++ b/qiita_db/metadata_template/prep_template.py @@ -207,6 +207,7 @@ def delete(cls, id_): ------ QiitaDBExecutionError If the prep template already has a preprocessed data + If the prep template has a raw data attached QiitaDBUnknownIDError If no prep template with id = id_ exists """ @@ -225,6 +226,17 @@ def delete(cls, id_): "because a preprocessed data has been" " already generated using it." % id_) + sql = """SELECT ( + SELECT raw_data_id + FROM qiita.prep_template + WHERE prep_template_id=%s) + IS NOT NULL""" + raw_data_attached = conn_handler.execute_fetchone(sql, (id_,))[0] + if raw_data_attached: + raise QiitaDBExecutionError( + "Cannot remove prep template %d because it has raw data " + "associated with it" % id_) + # Delete the prep template filepaths conn_handler.execute( "DELETE FROM qiita.prep_template_filepath WHERE " diff --git a/qiita_db/metadata_template/test/test_prep_template.py b/qiita_db/metadata_template/test/test_prep_template.py index e53c866b4..5fb3cd20a 100644 --- a/qiita_db/metadata_template/test/test_prep_template.py +++ b/qiita_db/metadata_template/test/test_prep_template.py @@ -1144,6 +1144,14 @@ def test_delete_unkonwn_id_error(self): with self.assertRaises(QiitaDBUnknownIDError): PrepTemplate.delete(5) + def test_delete_error_raw_data(self): + """Try to delete a prep template with a raw data attached to id""" + pt = PrepTemplate.create(self.metadata, self.test_study, + self.data_type_id) + pt.raw_data = RawData(1) + with self.assertRaises(QiitaDBExecutionError): + PrepTemplate.delete(pt.id) + def test_delete(self): """Deletes prep template 2""" pt = PrepTemplate.create(self.metadata, self.test_study, From 1057d7753041b596ee33cf96d8bb19662e058f6d Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Tue, 26 May 2015 13:27:51 -0500 Subject: [PATCH 5/5] Addressing @antgonza's comments --- qiita_db/metadata_template/prep_template.py | 2 +- qiita_db/metadata_template/test/test_prep_template.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qiita_db/metadata_template/prep_template.py b/qiita_db/metadata_template/prep_template.py index 45ab2c87e..610076f3f 100644 --- a/qiita_db/metadata_template/prep_template.py +++ b/qiita_db/metadata_template/prep_template.py @@ -108,7 +108,7 @@ def create(cls, md_template, study, data_type, investigation_type=None): # Get a connection handler conn_handler = SQLConnectionHandler() - queue_name = "CREATE_PREP_TEMPLATE_%d" % study.id + queue_name = "CREATE_PREP_TEMPLATE_%d_%d" % (study.id, id(md_template)) conn_handler.create_queue(queue_name) # Check if the data_type is the id or the string diff --git a/qiita_db/metadata_template/test/test_prep_template.py b/qiita_db/metadata_template/test/test_prep_template.py index 5fb3cd20a..615a4e19e 100644 --- a/qiita_db/metadata_template/test/test_prep_template.py +++ b/qiita_db/metadata_template/test/test_prep_template.py @@ -1342,7 +1342,7 @@ def test_raw_data_setter(self): self.data_type_id) self.assertEqual(pt.raw_data, None) pt.raw_data = rd - self.assertEqual(pt.raw_data, 1) + self.assertEqual(pt.raw_data, rd.id) EXP_PREP_TEMPLATE = (