From 88fbb593951116dd295e0f44de1e9c034ae92ad9 Mon Sep 17 00:00:00 2001 From: Bryan Brancotte Date: Tue, 25 Feb 2020 16:35:03 +0100 Subject: [PATCH 1/9] Allowing contributor to provide activity in mol, and computing the log10 value. Testing computation is done --- ippisite/ippidb/forms.py | 59 ++++++++++++++++++- ippisite/ippidb/tests_contribute.py | 90 ++++++++++++++++++++++++----- 2 files changed, 132 insertions(+), 17 deletions(-) diff --git a/ippisite/ippidb/forms.py b/ippisite/ippidb/forms.py index f702879d..41232a47 100644 --- a/ippisite/ippidb/forms.py +++ b/ippisite/ippidb/forms.py @@ -3,6 +3,8 @@ iPPI-DB django forms """ import itertools from collections import OrderedDict +from decimal import Decimal +from math import log10 from django import forms from django.contrib import messages @@ -15,7 +17,7 @@ from django.forms import ( modelformset_factory, formset_factory, inlineformset_factory, -) + widgets) from django.utils.translation import ugettext_lazy as _, ugettext from ippidb import ws, models @@ -817,10 +819,42 @@ class BaseInlineNestedFormSet(forms.BaseInlineFormSet): class CompoundActivityResultForm(ModelForm): compound_name = forms.ChoiceField(choices=(), required=True) + activity_mol = forms.DecimalField( + label="Activity", + required=False, + max_digits=12, + decimal_places=10, + min_value=0, + ) + activity_unit = forms.CharField( + label="Activity unit", + max_length=5, + required=False, + widget=widgets.Select( + choices=( + (None, "-----"), + ("1", "mol"), + ("1e-3", "mmol"), + ("1e-6", "µmol"), + ("1e-9", "nmol"), + ("1e-12", "pmol"), + ), + ) + ) class Meta: model = models.CompoundActivityResult - fields = ("compound_name", "activity_type", "activity", "modulation_type") + fields = ( + "compound_name", + "modulation_type", + "activity", + "activity_type", + "activity_mol", + "activity_unit", + ) + widgets = { + "activity": widgets.HiddenInput(), + } def has_changed(self): """ @@ -837,6 +871,27 @@ class CompoundActivityResultForm(ModelForm): return False return super().has_changed() + def clean(self): + cleaned_data = super().clean() + if cleaned_data["activity_mol"] == 0: + self.add_error("activity_mol", "Must be greater than 0") + return cleaned_data + if cleaned_data["activity_mol"] is None: + return cleaned_data + if cleaned_data["activity_unit"] in (None, ""): + self.add_error("activity_unit", "Unit must be provided") + return cleaned_data + try: + d = Decimal(-log10( + Decimal(self.cleaned_data["activity_mol"]) * Decimal(self.cleaned_data["activity_unit"]) + )) + d = d.quantize(Decimal(10) ** -self.instance._meta.get_field("activity").decimal_places) + self.cleaned_data["activity"] = d + except Exception as e: + self.add_error("activity_mol", "Unknown error when computing -log10(activity): %s" % str(e)) + self.add_error("activity_unit", "Unknown error when computing -log10(activity): %s" % str(e)) + return cleaned_data + def save(self, commit=True): # right before an actual save, we set the foreign key that have been created in the meantime from unique # identifier (compound_name) we where provided at the initialization of the form. diff --git a/ippisite/ippidb/tests_contribute.py b/ippisite/ippidb/tests_contribute.py index 385dcefd..1a0331d0 100644 --- a/ippisite/ippidb/tests_contribute.py +++ b/ippisite/ippidb/tests_contribute.py @@ -1,6 +1,8 @@ """ iPPI-DB contribution module tests """ +import math +from decimal import Decimal from tempfile import NamedTemporaryFile from django.contrib.auth import get_user_model @@ -159,6 +161,14 @@ class ContributionViewsTestCase(TestCase): "Pharmacokinetic tests count", ), ] + for activity_tests in entry_data["activity_tests"]: + for compound_activity_results in activity_tests["compound_activity_results"]: + if "activity_mol" in compound_activity_results: + compound_activity_results["activity"] = -math.log10( + Decimal(str(compound_activity_results["activity_mol"])) + * Decimal(str(compound_activity_results["activity_unit"])) + ) + self._process_contribution_wizard_without_sanity_check(entry_data) for fcn, results, msg in future_expected_equals: self.assertEqual(fcn(), results, msg=msg) @@ -370,12 +380,15 @@ class ContributionViewsTestCase(TestCase): data[ f"{idx}-compoundactivityresult_set-activity-results-{nidx}-activity_type" ] = compound_activity_result["activity_type"] - data[ - f"{idx}-compoundactivityresult_set-activity-results-{nidx}-activity" - ] = compound_activity_result["activity"] - data[ - f"{idx}-compoundactivityresult_set-activity-results-{nidx}-inhibition_percentage" - ] = compound_activity_result["inhibition_percentage"] + try: + data[ + f"{idx}-compoundactivityresult_set-activity-results-{nidx}-activity_mol" + ] = compound_activity_result["activity_mol"] + data[ + f"{idx}-compoundactivityresult_set-activity-results-{nidx}-activity_unit" + ] = compound_activity_result["activity_unit"] + except KeyError: + pass data[ f"{idx}-compoundactivityresult_set-activity-results-{nidx}-modulation_type" ] = compound_activity_result["modulation_type"] @@ -500,11 +513,11 @@ class ContributionViewsTestCase(TestCase): } yield item - def test_basic_entry(self): + def get_basic_entry(self): """ Basic entry test """ - entry_data = { + return { "source": "PM", "id_source": "15072770", "in_silico": True, @@ -553,7 +566,8 @@ class ContributionViewsTestCase(TestCase): { "compound_name": "toto", "activity_type": "pIC50", - "activity": 6.85, + "activity_mol": 6.85, + "activity_unit": "1e-6", "inhibition_percentage": "", "modulation_type": "I", } @@ -561,7 +575,52 @@ class ContributionViewsTestCase(TestCase): } ], } - self._process_contribution_wizard(entry_data) + + def test_basic_entry(self): + self._process_contribution_wizard(self.get_basic_entry()) + + def _test_activity_computation_and_storage(self, value, power): + basic_entry = self.get_basic_entry() + for activity_tests in basic_entry["activity_tests"]: + for compound_activity_results in activity_tests["compound_activity_results"]: + compound_activity_results["activity_unit"] = "1e-%i" % power + try: + del compound_activity_results["activity"] + except KeyError: + pass + self._process_contribution_wizard(basic_entry) + bibliography = models.Bibliography.objects.get(id_source=basic_entry["id_source"]) + for activity_tests in basic_entry["activity_tests"]: + for compound_activity_results in activity_tests["compound_activity_results"]: + compound = models.RefCompoundBiblio.objects.get( + bibliography=bibliography, + compound_name=compound_activity_results["compound_name"] + ).compound + results = models.CompoundActivityResult.objects.get( + activity_type=compound_activity_results["activity_type"], + modulation_type=compound_activity_results["modulation_type"], + compound=compound + ) + self.assertEqual( + Decimal(compound_activity_results["activity"]).quantize( + Decimal(10) ** -models.CompoundActivityResult._meta.get_field("activity").decimal_places + ), + results.activity) + + def test_activity_computation_and_storage(self): + self._test_activity_computation_and_storage(0.685, 0) + + def test_activity_computation_and_storage_3(self): + self._test_activity_computation_and_storage(639.39406, 3) + + def test_activity_computation_and_storage_6(self): + self._test_activity_computation_and_storage(4.9846073, 6) + + def test_activity_computation_and_storage_9(self): + self._test_activity_computation_and_storage(2380.002, 9) + + def test_activity_computation_and_storage_12(self): + self._test_activity_computation_and_storage(1.018, 12) def test_with_all_tests(self): """ @@ -618,8 +677,8 @@ class ContributionViewsTestCase(TestCase): { "compound_name": "toto", "activity_type": "pIC50", - "activity": 6.85, - "inhibition_percentage": "", + "activity_mol": 6.85, + "activity_unit": "1e-3", "modulation_type": "I", } ], @@ -730,7 +789,8 @@ class ContributionViewsTestCase(TestCase): { "compound_name": "toto", "activity_type": "pIC50", - "activity": 6.85, + "activity_mol": 6.85, + "activity_unit": "1e-3", "inhibition_percentage": "", "modulation_type": "I", } @@ -810,7 +870,8 @@ class ContributionViewsTestCase(TestCase): { "compound_name": "16d", "activity_type": "pIC50", - "activity": 6.85, + "activity_mol": 6.85, + "activity_unit": "1e-3", "inhibition_percentage": "", "modulation_type": "I", } @@ -879,7 +940,6 @@ class ContributionViewsTestCase(TestCase): { "compound_name": "FC", "activity_type": "pIC50", - "activity": "", "inhibition_percentage": "", "modulation_type": "S", } -- GitLab From 89a4b78bb488544d3b1b27714f4f83fc14d79bcf Mon Sep 17 00:00:00 2001 From: Bryan Brancotte Date: Tue, 25 Feb 2020 16:35:36 +0100 Subject: [PATCH 2/9] More comments in case of error --- ippisite/ippidb/tests_contribute.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ippisite/ippidb/tests_contribute.py b/ippisite/ippidb/tests_contribute.py index 1a0331d0..627105d7 100644 --- a/ippisite/ippidb/tests_contribute.py +++ b/ippisite/ippidb/tests_contribute.py @@ -135,7 +135,7 @@ class ContributionViewsTestCase(TestCase): ( models.Compound.objects.validated().count, models.Compound.objects.validated().count(), - "Validated Compounds count", + "Validated Compounds count should remains the same", ), ( models.CompoundAction.objects.count, @@ -176,7 +176,7 @@ class ContributionViewsTestCase(TestCase): ( models.Compound.objects.validated().count, models.Compound.objects.count(), - "Validated Compounds count", + "Validated Compounds count should have increased", ) ] contribution_to_be_validated = models.Contribution.objects.get(validated=False) -- GitLab From aa63555b9a7ee93c14abbfb82fd7e9aa05763dbb Mon Sep 17 00:00:00 2001 From: Bryan Brancotte Date: Tue, 25 Feb 2020 16:47:40 +0100 Subject: [PATCH 3/9] fixing formatting --- ippisite/ippidb/forms.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ippisite/ippidb/forms.py b/ippisite/ippidb/forms.py index 41232a47..4f19f7e9 100644 --- a/ippisite/ippidb/forms.py +++ b/ippisite/ippidb/forms.py @@ -17,7 +17,8 @@ from django.forms import ( modelformset_factory, formset_factory, inlineformset_factory, - widgets) + widgets, +) from django.utils.translation import ugettext_lazy as _, ugettext from ippidb import ws, models -- GitLab From e9158a7616d1e2c2b1289b41f24f1daea8ee77b5 Mon Sep 17 00:00:00 2001 From: Bryan Brancotte Date: Tue, 25 Feb 2020 17:04:26 +0100 Subject: [PATCH 4/9] linting with flake8 only --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d375ed1d..3fd6a243 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -7,7 +7,7 @@ test-style: script: - apt update && apt install -y apache2-dev graphviz graphviz-dev - cd ippisite - - pip install -r requirements-dev.txt + - pip install flake8 - flake8 --config=.flake8 test-ansible: image: python:3.5 @@ -121,4 +121,4 @@ deploy-webserver-production: - ansible-playbook -vvv -i ./hosts_release deploy.yaml --extra-vars "deploy_user_name=ippidb repo_api_token=JZS-4cH7bWkFkHa2rAVf marvinjs_apikey=$MARVINJS_APIKEY_release galaxy_base_url=$GALAXY_BASE_URL_release galaxy_apikey=$GALAXY_APIKEY_release galaxy_compoundproperties_workflowid=$GALAXY_COMPOUNDPROPERTIES_WORKFLOWID_release secret_key=$SECRET_KEY_release dbname=$DBNAME_release dbuser=$DBUSER_release dbpassword=$DBPASSWORD_release dbhost=$DBHOST_release dbport=$DBPORT_release http_port=$HTTP_PORT_release branch=$CI_COMMIT_REF_NAME" only: - - release \ No newline at end of file + - release -- GitLab From 66e41ef3928d304eac21396350c8b988494fcb74 Mon Sep 17 00:00:00 2001 From: Bryan Brancotte Date: Wed, 26 Feb 2020 10:56:02 +0100 Subject: [PATCH 5/9] Making activity required --- ippisite/ippidb/forms.py | 4 ++-- ippisite/ippidb/tests_contribute.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ippisite/ippidb/forms.py b/ippisite/ippidb/forms.py index 4f19f7e9..ba38709f 100644 --- a/ippisite/ippidb/forms.py +++ b/ippisite/ippidb/forms.py @@ -822,7 +822,7 @@ class CompoundActivityResultForm(ModelForm): compound_name = forms.ChoiceField(choices=(), required=True) activity_mol = forms.DecimalField( label="Activity", - required=False, + required=True, max_digits=12, decimal_places=10, min_value=0, @@ -830,7 +830,7 @@ class CompoundActivityResultForm(ModelForm): activity_unit = forms.CharField( label="Activity unit", max_length=5, - required=False, + required=True, widget=widgets.Select( choices=( (None, "-----"), diff --git a/ippisite/ippidb/tests_contribute.py b/ippisite/ippidb/tests_contribute.py index 627105d7..4d6ef2fa 100644 --- a/ippisite/ippidb/tests_contribute.py +++ b/ippisite/ippidb/tests_contribute.py @@ -940,7 +940,8 @@ class ContributionViewsTestCase(TestCase): { "compound_name": "FC", "activity_type": "pIC50", - "inhibition_percentage": "", + "activity_mol": 0.25, + "activity_unit": "1e-6", "modulation_type": "S", } ], -- GitLab From 29b3c361b26486e06bd8bf7c15115cda3573719a Mon Sep 17 00:00:00 2001 From: Bryan Brancotte Date: Wed, 26 Feb 2020 10:56:19 +0100 Subject: [PATCH 6/9] removing inhibition_percentage from tests --- ippisite/ippidb/tests_contribute.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/ippisite/ippidb/tests_contribute.py b/ippisite/ippidb/tests_contribute.py index 4d6ef2fa..143cb665 100644 --- a/ippisite/ippidb/tests_contribute.py +++ b/ippisite/ippidb/tests_contribute.py @@ -568,7 +568,6 @@ class ContributionViewsTestCase(TestCase): "activity_type": "pIC50", "activity_mol": 6.85, "activity_unit": "1e-6", - "inhibition_percentage": "", "modulation_type": "I", } ], @@ -791,7 +790,6 @@ class ContributionViewsTestCase(TestCase): "activity_type": "pIC50", "activity_mol": 6.85, "activity_unit": "1e-3", - "inhibition_percentage": "", "modulation_type": "I", } ], @@ -872,7 +870,6 @@ class ContributionViewsTestCase(TestCase): "activity_type": "pIC50", "activity_mol": 6.85, "activity_unit": "1e-3", - "inhibition_percentage": "", "modulation_type": "I", } ], -- GitLab From 215ff2ea9fcd1da41212c345bf2bbe63f6813713 Mon Sep 17 00:00:00 2001 From: Bryan Brancotte Date: Wed, 26 Feb 2020 11:32:51 +0100 Subject: [PATCH 7/9] raising error when activity_mol is not provided --- ippisite/ippidb/forms.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ippisite/ippidb/forms.py b/ippisite/ippidb/forms.py index ba38709f..930375a1 100644 --- a/ippisite/ippidb/forms.py +++ b/ippisite/ippidb/forms.py @@ -874,6 +874,9 @@ class CompoundActivityResultForm(ModelForm): def clean(self): cleaned_data = super().clean() + if "activity_mol" not in cleaned_data: + self.add_error("activity_mol", "Must be provided") + return cleaned_data if cleaned_data["activity_mol"] == 0: self.add_error("activity_mol", "Must be greater than 0") return cleaned_data -- GitLab From ecc064577c76c475716475b6fb83b345b5e3dbf6 Mon Sep 17 00:00:00 2001 From: Bryan Brancotte Date: Wed, 26 Feb 2020 11:39:07 +0100 Subject: [PATCH 8/9] ensuring that the validation fails when no response_mol is provided --- ippisite/ippidb/tests_contribute.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/ippisite/ippidb/tests_contribute.py b/ippisite/ippidb/tests_contribute.py index 143cb665..70151c86 100644 --- a/ippisite/ippidb/tests_contribute.py +++ b/ippisite/ippidb/tests_contribute.py @@ -163,7 +163,7 @@ class ContributionViewsTestCase(TestCase): ] for activity_tests in entry_data["activity_tests"]: for compound_activity_results in activity_tests["compound_activity_results"]: - if "activity_mol" in compound_activity_results: + if "activity_mol" in compound_activity_results and compound_activity_results["activity_mol"] != "": compound_activity_results["activity"] = -math.log10( Decimal(str(compound_activity_results["activity_mol"])) * Decimal(str(compound_activity_results["activity_unit"])) @@ -621,6 +621,21 @@ class ContributionViewsTestCase(TestCase): def test_activity_computation_and_storage_12(self): self._test_activity_computation_and_storage(1.018, 12) + def test_no_activity_fails(self): + basic_entry = self.get_basic_entry() + for activity_tests in basic_entry["activity_tests"]: + for compound_activity_results in activity_tests["compound_activity_results"]: + compound_activity_results["activity_unit"] = "" + compound_activity_results["activity_mol"] = "" + try: + del compound_activity_results["activity"] + except KeyError: + pass + self._process_contribution_wizard_without_sanity_check( + basic_entry, + error_expected_in_step="ActivityDescriptionFormSet", + ) + def test_with_all_tests(self): """ Basic entry test -- GitLab From d6e7f94671fdcde82d430246bb6586df03a7b633 Mon Sep 17 00:00:00 2001 From: Bryan Brancotte Date: Wed, 26 Feb 2020 11:40:27 +0100 Subject: [PATCH 9/9] Allow to test the contribution wizard for failure at a specific step --- ippisite/ippidb/tests_contribute.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ippisite/ippidb/tests_contribute.py b/ippisite/ippidb/tests_contribute.py index 70151c86..dd93aad1 100644 --- a/ippisite/ippidb/tests_contribute.py +++ b/ippisite/ippidb/tests_contribute.py @@ -185,7 +185,7 @@ class ContributionViewsTestCase(TestCase): for fcn, results, msg in post_validation_expected_equals: self.assertEqual(fcn(), results, msg=msg) - def _process_contribution_wizard_without_sanity_check(self, entry_data): + def _process_contribution_wizard_without_sanity_check(self, entry_data, error_expected_in_step=None): """ The contribution add "wizard" returns a 200 @@ -209,13 +209,17 @@ class ContributionViewsTestCase(TestCase): } form_data["ippi_wizard-current_step"] = step["step-id"] response = self.client.post(step_url, form_data) - if response.status_code != 302 and step.get("step-id") != "done": + error_is_now = error_expected_in_step == step.get("step-id") and response.status_code != 302 + if response.status_code != 302 and step.get("step-id") != "done" or error_is_now: file_path = self.write_in_tmp_file(response) err_msg = ( f"Response code not ok when getting form for " f"{step.get('step-id')} at {step_url}. Server" f" response is stored in {file_path}" ) + if error_is_now: + self.assertEqual(response.status_code, 200, err_msg) + return self.assertEqual(response.status_code, 302, err_msg) # check redirect to next step (if there is one) if step.get("next") is not None: -- GitLab