Devel validation #103

dalonsoa · 2023-06-09T04:10:49Z

Simply to monitor the evolution of changes when updating the validation tooling.

…ess.

…fixed

…). 2) Cleaning up code

dalonsoa

@pxjacomex , I've left here a few comments for you to check.

We really need to work with small chunks of work - having to review over 100 files is just impracticable for an efficient development process.

dalonsoa · 2023-06-09T04:14:37Z

daily/__init__.py

@pxjacomex , describe here in a docstring what the purpose of this app is, so it is easier to understand its contents.

daily/models.py

dalonsoa · 2023-06-09T04:35:49Z

daily/models.py

+        abstract = True
+
+
+def create_Dai_model(digits=14, decimals=6, fields=("Average")) -> Type[TimescaleModel]:


Suggested change

def create_Dai_model(digits=14, decimals=6, fields=("Average")) -> Type[TimescaleModel]:

def create_day_model(digits=14, decimals=6, fields=("Average")) -> Type[TimescaleModel]:

You will need to update everywhere this function is used.

daily/models.py

dalonsoa · 2023-06-09T07:16:21Z

daily/filters.py

@@ -0,0 +1,32 @@
+from django_filters import rest_framework as filters
+
+from station.models import Station


This import is not needed here

dalonsoa · 2023-06-09T07:20:30Z

daily/models.py

+class PercentageOxygenConcentrationDepth(
+    create_Dai_model(
+        digits=6,
+        decimals=2,
+    ),
+):
+    """Percentage oxygen concentration (mg/l) at a depth in cm.
+
+    HELPWANTED: Is this wrong? It's teh same as above, perhaps units should
+    be %? --> DIEGO: Looks identical to the previous one to me. It might be an error.
+    """
+
+    depth = models.PositiveSmallIntegerField("Depth")
+
+    class Meta:
+        default_permissions = ()
+        indexes = [
+            models.Index(fields=["station_id", "depth", "time"]),
+        ]


Could you comment on this one? It looks identical to the previous class.

dalonsoa · 2023-06-09T07:20:43Z

daily/models.py

+        decimals=2,
+    ),
+):
+    """Phycocyanin (?) at a depth in cm."""


What are the units for this?

…ly after validation

dalonsoa

A few more comments from my side.

dalonsoa · 2023-06-22T06:47:04Z

daily/models.py

+        abstract = True
+
+
+def create_Dai_model(


Dai does not mean anything in english. Make the following change and change wherever is used.

Suggested change

def create_Dai_model(

def create_day_model(

dalonsoa · 2023-06-22T06:47:26Z

daily/models.py

+    attrs.update(_fields)
+
+    return type(
+        f"_Dai{num}",


Suggested change

f"_Dai{num}",

f"_Day{num}",

dalonsoa · 2023-06-22T06:50:19Z

daily/serializers.py

+# TODO Confirm if DischargeCurveSerializer is not needed in Validated Models
+# class DischargeCurveSerializer(serializers.ModelSerializer):
+#     class Meta:
+#         model = DischargeCurve
+#         exclude = []
+
+
+# class LevelFunctionSerializer(serializers.ModelSerializer):
+#     class Meta:
+#         model = LevelFunction
+#         exclude = []
+


Who needs to confirm this?

dalonsoa · 2023-06-22T06:52:13Z

daily/urls.py

+
+from . import views
+
+app_name = "hourly"


This is the Daily app, right?

Suggested change

app_name = "hourly"

app_name = "daily"

dalonsoa · 2023-06-22T06:56:47Z

daily/views.py

+########################################################################################
+# TODO: Revisit theses specialised views that use level_function_table() and create
+# Django Rest Framework equivalents.
+########################################################################################


Rather than leaving this in the code commented, remove all of these code and open an issue about it, giving also some context about why it needs to be revisited.

dalonsoa · 2023-06-22T06:58:33Z

djangomain/renderers.py

Why is this empty file needed? Remove if not needed.

dalonsoa · 2023-06-22T06:59:30Z

formatting/serializers.py

Why is this empty file needed? Remove if not needed.

dalonsoa · 2023-06-22T07:02:24Z

hourly/filters.py

@@ -0,0 +1,32 @@
+from django_filters import rest_framework as filters
+
+from station.models import Station


Import not needed here.

dalonsoa · 2023-06-22T07:11:16Z

hourly/filters.py

+class HourlyFilter(filters.FilterSet):
+    """
+    Filter class for hourlys that are not Polar Wind, Discharge Curve, or Level Function
+    and that have no depth information.
+    """
+
+    time = filters.DateTimeFilter(field_name="time", lookup_expr="exact")
+    min_time = filters.DateTimeFilter(field_name="time", lookup_expr="gte")
+    max_time = filters.DateTimeFilter(field_name="time", lookup_expr="lte")
+    value = filters.NumberFilter(field_name="value", lookup_expr="exact")
+    min_value = filters.NumberFilter(field_name="value", lookup_expr="gte")
+    max_value = filters.NumberFilter(field_name="value", lookup_expr="lte")
+    station_id = filters.NumberFilter(field_name="station_id", lookup_expr="exact")
+    used_for_hourly = filters.BooleanFilter(
+        fieldname="used_for_hourly", lookup_expr="exact"
+    )
+
+
+class HourlyFilterDepth(HourlyFilter):


These filters are identical to the daily filters and the monthly filters. If they need to be defined on an app basis, I would suggest to:

Create utilities/filters.py and put all this filters within JournalFilter and JournalFilterDepth classes.

In this file and the corresponding ones in the daily and monthly apps, subclass them:

from utilities.filters import JournalFilter, JournalFilterDepth class HourlyFilter(JournalFilter): """ Filter class for hourlys that are not Polar Wind, Discharge Curve, or Level Function and that have no depth information. """ class HourlyFilterDepth(JournalFilterDepth): """ Filter class for hourlys that are not Polar Wind, Discharge Curve, or Level Function and that have depth information. """

However, if they do not need to be defined on an app basis, just skip step 2 altogether and directly import the JournalFilter and JournalFilterDepth where relevant. The more code we write, the harder it is to maintain, so let's keep it to a minimum.

dalonsoa · 2023-06-22T07:12:10Z

hourly/models.py

+"""Available hourly variables."""
+
+
+# TODO check if PolarWind is needed in HOurly


To check with whom? If unsure, just delete these code and open an issue with it.

…d from grand parent class to parent)

…izing

…ge, sum and value

dalonsoa · 2023-08-03T15:29:54Z

daily/models.py

+from django.urls import reverse
+from timescale.db.models.models import TimescaleModel
+
+from station.models import Station


Suggested change

from station.models import Station

pxjacomex added 15 commits April 14, 2023 10:24

Models for Validated, Hourly, Daily and Monthly

d95a83a

Added initial migrations

5092539

Validation calculations and UI. First approach for integration

a5d27f3

Some changes

02476a7

Updates for validation calculations

ca9badf

Delete .idea directory, added to .gitignore

5955570

Pre-commit test: Updating versions of black and isort for pre-commit

817442f

Validation module: Calculations for detail of the day, and store proc…

b19e84d

…ess.

Validation: GUI (partial progress)

9887a8d

Validation module GUI: Creating Tab panels for plot and tables

b972042

Validation: GUI translation to english first steps. Some python code …

e28c6dc

…fixed

Validation module: 1) translation of python and js functions (partial…

b611d82

…). 2) Cleaning up code

translating to english

7e71dbd

Translation JS and python (partial)

fdb84d8

Validation module: translation

6a2324a

dalonsoa commented Jun 9, 2023

View reviewed changes

pxjacomex added 2 commits June 11, 2023 22:14

1) GUI for downloading and plot data 2) Function for calculating Hour…

4de8511

…ly after validation

Validatin module: Calculte Daily, Monthly and Hourly in Thread launch

24e8b7e

dalonsoa commented Jun 22, 2023

View reviewed changes

pxjacomex added 5 commits June 22, 2023 20:20

1)Added Plotly JS file, 2)Indexes definitions in parent Class (change…

70e712f

…d from grand parent class to parent)

Validation: Temporal code: changing column name to 'value' for normal…

c0250db

…izing

Generalization of validation module for different column names: avera…

3205428

…ge, sum and value

Validation module updates

d8cc67c

Validation module Ver. 1.0

d5dede4

dalonsoa marked this pull request as ready for review July 21, 2023 13:59

dalonsoa and others added 3 commits July 21, 2023 14:59

Merge branch 'develop' into devel-validation

38c04c3

Add extra initial data and adatp to use under Windows

88c2da2

Update docker configuration

d1fcd84

dalonsoa mentioned this pull request Aug 3, 2023

Simplify filters to avoid duplication #109

Closed

dalonsoa commented Aug 3, 2023

View reviewed changes

dalonsoa merged commit 8c2e2b8 into develop Aug 3, 2023
4 of 5 checks passed

dalonsoa deleted the devel-validation branch August 3, 2023 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Devel validation #103

Devel validation #103

dalonsoa commented Jun 9, 2023

dalonsoa left a comment

dalonsoa Jun 9, 2023

dalonsoa Jun 9, 2023

dalonsoa Jun 9, 2023

dalonsoa Jun 9, 2023

dalonsoa Jun 9, 2023

dalonsoa left a comment

dalonsoa Jun 22, 2023

dalonsoa Jun 22, 2023

dalonsoa Jun 22, 2023

dalonsoa Jun 22, 2023

dalonsoa Jun 22, 2023

dalonsoa Jun 22, 2023

dalonsoa Jun 22, 2023

dalonsoa Jun 22, 2023

dalonsoa Jun 22, 2023

dalonsoa Jun 22, 2023

dalonsoa Aug 3, 2023

		abstract = True


		def create_Dai_model(digits=14, decimals=6, fields=("Average")) -> Type[TimescaleModel]:

	def create_Dai_model(digits=14, decimals=6, fields=("Average")) -> Type[TimescaleModel]:
	def create_day_model(digits=14, decimals=6, fields=("Average")) -> Type[TimescaleModel]:

		@@ -0,0 +1,32 @@
		from django_filters import rest_framework as filters

		from station.models import Station

		"""Available hourly variables."""


		# TODO check if PolarWind is needed in HOurly

Devel validation #103

Devel validation #103

Conversation

dalonsoa commented Jun 9, 2023

dalonsoa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dalonsoa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment