Skip to content

Adding check_restrictions functionality #1141

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 4 commits into from
May 5, 2015
Merged

Adding check_restrictions functionality #1141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7cf9fd6
Adding check_restrictions functionality and tests
josenavas May 1, 2015
7c9089e
pep8ing...
josenavas May 2, 2015
55944f2
Using set operations
josenavas May 2, 2015
20eebdb
Addressing @wasade's comment
josenavas May 4, 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
18 changes: 18 additions & 0 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 @@ -1157,3 +1157,21 @@ def update_category(self, category, samples_and_values):
% (category, value_str, value_types_str, column_type))

raise e

def check_restrictions(self, restrictions):
"""Checks if the template fulfills the restrictions

Parameters
----------
restrictions : list of Restriction
The restrictions to test if the template fulfills

Returns
-------
set of str
The missing columns
"""
cols = {col for restriction in restrictions
for col in restriction.columns}

Copy link
Contributor

@wasade wasade May 4, 2015

Choose a reason for hiding this comment

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

Why not just do {col for restriction in restrictions for col in restriction.columns}? The generator is immediately consumed so it isn't clear what the benefit of lazy eval is here

Copy link
Contributor Author

@josenavas josenavas May 4, 2015

Choose a reason for hiding this comment

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

I was just avoiding the double list comprehension, as sometimes it has been criticized. I think it is not too complicated, so I'll change it.

Copy link
Contributor

@wasade wasade May 4, 2015

Choose a reason for hiding this comment

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

makes sense. in that case though, it would be better to just create the set
using two for loops. eg

cols = set()
for restriction in restrictions:
for col in restriction.columns:
cols.add(col)

On Mon, May 4, 2015 at 2:29 PM, josenavas notifications@github.com wrote:

In qiita_db/metadata_template/base_metadata_template.py
#1141 (comment):

  •    Parameters

  •    restrictions : list of Restriction

  •        The restrictions to test if the template fulfills

  •    Returns

  •    set of str

  •        The missing columns

  •    """

  •    def _col_iter():

  •        for restriction in restrictions:

  •            for col in restriction.columns:

  •                yield col

I was just avoiding the double list comprehension, as sometimes it has
been criticized. I think it is not too complicated, so I'll change it.

—
Reply to this email directly or view it on GitHub
https://github.com/biocore/qiita/pull/1141/files#r29620275.

return cols.difference(self.categories())
17 changes: 16 additions & 1 deletion 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 @@ -32,7 +32,8 @@
get_count)
from qiita_db.metadata_template.prep_template import PrepTemplate, PrepSample
from qiita_db.metadata_template.sample_template import SampleTemplate, Sample
from qiita_db.metadata_template.constants import PREP_TEMPLATE_COLUMNS
from qiita_db.metadata_template.constants import (
PREP_TEMPLATE_COLUMNS, PREP_TEMPLATE_COLUMNS_TARGET_GENE)


class BaseTestPrepSample(TestCase):
Expand Down Expand Up @@ -1299,6 +1300,20 @@ def test_qiime_map_fp(self):
'1_prep_1_qiime_19700101-000000.txt')
self.assertEqual(pt.qiime_map_fp, exp)

def test_check_restrictions(self):
obs = self.tester.check_restrictions([PREP_TEMPLATE_COLUMNS['EBI']])
self.assertEqual(obs, set())

del self.metadata['primer']
pt = npt.assert_warns(QiitaDBWarning, PrepTemplate.create,
self.metadata, self.new_raw_data,
self.test_study, self.data_type)

obs = pt.check_restrictions(
[PREP_TEMPLATE_COLUMNS['EBI'],
PREP_TEMPLATE_COLUMNS_TARGET_GENE['demultiplex']])
self.assertEqual(obs, {'primer'})


EXP_PREP_TEMPLATE = (
'sample_name\tbarcode\tcenter_name\tcenter_project_name\t'
Expand Down
10 changes: 10 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 @@ -1955,6 +1955,16 @@ def test_to_dataframe(self):
'anonymized_name', 'tot_org_carb', 'description_duplicate',
'env_feature'})

def test_check_restrictions(self):
obs = self.tester.check_restrictions([SAMPLE_TEMPLATE_COLUMNS['EBI']])
self.assertEqual(obs, set())

del self.metadata['collection_timestamp']
st = npt.assert_warns(QiitaDBWarning, SampleTemplate.create,
self.metadata, self.new_study)
obs = st.check_restrictions([SAMPLE_TEMPLATE_COLUMNS['EBI']])
self.assertEqual(obs, {'collection_timestamp'})

EXP_SAMPLE_TEMPLATE = (
"sample_name\tcollection_timestamp\tdescription\tdna_extracted"
"\thost_subject_id\tint_column\tlatitude\tlongitude"
Expand Down