From 1e994cf7856e20acc0b058a3b6f0f617f8113726 Mon Sep 17 00:00:00 2001 From: k1o0 Date: Tue, 13 Feb 2024 16:49:06 +0200 Subject: [PATCH 1/9] Resolves #827 --- alyx/actions/admin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/alyx/actions/admin.py b/alyx/actions/admin.py index 73b1c959..c050063a 100644 --- a/alyx/actions/admin.py +++ b/alyx/actions/admin.py @@ -390,7 +390,8 @@ def is_water_restricted(self, obj): class WeighingForm(BaseActionForm): def __init__(self, *args, **kwargs): super(WeighingForm, self).__init__(*args, **kwargs) - self.fields['subject'].queryset = self.current_user.get_allowed_subjects() + if 'subject' in self.fields: + self.fields['subject'].queryset = self.current_user.get_allowed_subjects() if self.fields.keys(): self.fields['weight'].widget.attrs.update({'autofocus': 'autofocus'}) From eb27efe295fd564e9b7781e6e6ba39832c33d559 Mon Sep 17 00:00:00 2001 From: Miles Wells Date: Wed, 14 Feb 2024 17:20:58 +0200 Subject: [PATCH 2/9] Issue #830 --- alyx/actions/admin.py | 6 +++--- alyx/actions/models.py | 16 ++++++--------- alyx/actions/serializers.py | 3 ++- alyx/actions/tests_rest.py | 20 ++++++++++++++++-- alyx/actions/views.py | 20 +++++++++++++++--- alyx/alyx/__init__.py | 2 +- alyx/data/admin.py | 9 ++++---- alyx/data/migrations/0019_dataset_qc.py | 18 ++++++++++++++++ alyx/data/models.py | 5 +++++ alyx/data/serializers.py | 4 +++- alyx/data/views.py | 2 ++ alyx/experiments/serializers.py | 2 +- alyx/experiments/tests_rest.py | 24 +++++++++++++++++++--- alyx/experiments/views.py | 13 +++++++++++- alyx/misc/management/commands/one_cache.py | 9 ++++++-- requirements.txt | 2 +- 16 files changed, 122 insertions(+), 33 deletions(-) create mode 100644 alyx/data/migrations/0019_dataset_qc.py diff --git a/alyx/actions/admin.py b/alyx/actions/admin.py index c050063a..f764ab5c 100644 --- a/alyx/actions/admin.py +++ b/alyx/actions/admin.py @@ -456,10 +456,10 @@ class DatasetInline(BaseInlineAdmin): show_change_link = True model = Dataset extra = 1 - fields = ('name', 'dataset_type', 'collection', '_online', 'version', 'created_by', - 'created_datetime') + fields = ('name', 'dataset_type', 'collection', '_online', 'version', 'qc', + 'created_by', 'created_datetime') readonly_fields = fields - ordering = ("name",) + ordering = ('name',) def _online(self, obj): return obj.is_online diff --git a/alyx/actions/models.py b/alyx/actions/models.py index 573696f9..edec7c74 100644 --- a/alyx/actions/models.py +++ b/alyx/actions/models.py @@ -1,7 +1,9 @@ from datetime import timedelta -import structlog from math import inf +import structlog +from one.alf.spec import QC + from django.conf import settings from django.core.validators import MinValueValidator from django.db import models @@ -253,16 +255,10 @@ class Session(BaseAction): n_trials = models.IntegerField(blank=True, null=True) n_correct_trials = models.IntegerField(blank=True, null=True) - QC_CHOICES = [ - (50, 'CRITICAL',), - (40, 'FAIL',), - (30, 'WARNING',), - (0, 'NOT_SET',), - (10, 'PASS',), - ] - - qc = models.IntegerField(default=0, choices=QC_CHOICES, + QC_CHOICES = [(e.value, e.name) for e in QC] + qc = models.IntegerField(default=QC.NOT_SET, choices=QC_CHOICES, help_text=' / '.join([str(q[0]) + ': ' + q[1] for q in QC_CHOICES])) + extended_qc = models.JSONField(null=True, blank=True, help_text="Structured data about session QC," "formatted in a user-defined way") diff --git a/alyx/actions/serializers.py b/alyx/actions/serializers.py index 26f37417..0809e779 100644 --- a/alyx/actions/serializers.py +++ b/alyx/actions/serializers.py @@ -98,12 +98,13 @@ class SessionDatasetsSerializer(serializers.ModelSerializer): queryset=DatasetType.objects.all(), ) default_revision = serializers.CharField(source='default_dataset') + qc = BaseSerializerEnumField(required=False) class Meta: list_serializer_class = FilterDatasetSerializer model = Dataset fields = ('id', 'name', 'dataset_type', 'data_url', 'url', 'file_size', - 'hash', 'version', 'collection', 'revision', 'default_revision') + 'hash', 'version', 'collection', 'revision', 'default_revision', 'qc') class SessionWaterAdminSerializer(serializers.ModelSerializer): diff --git a/alyx/actions/tests_rest.py b/alyx/actions/tests_rest.py index 4fe66f4f..8bf83c2a 100644 --- a/alyx/actions/tests_rest.py +++ b/alyx/actions/tests_rest.py @@ -266,8 +266,8 @@ def test_sessions(self): # test dataset type filters dtype1, _ = DatasetType.objects.get_or_create(name='trials.table') dtype2, _ = DatasetType.objects.get_or_create(name='wheel.position') - Dataset.objects.create(session=ses, name='_ibl_trials.table.pqt', dataset_type=dtype1) - Dataset.objects.create(session=ses, name='_ibl_wheel.position.npy', dataset_type=dtype2) + Dataset.objects.create(session=ses, name='_ibl_trials.table.pqt', dataset_type=dtype1, qc=40) + Dataset.objects.create(session=ses, name='_ibl_wheel.position.npy', dataset_type=dtype2, qc=30) d = self.ar(self.client.get(reverse('session-list') + '?dataset_types=wheel.position')) self.assertCountEqual([str(ses.pk)], (x['id'] for x in d)) q = '?dataset_types=wheel.position,trials.table' # Check with list @@ -280,6 +280,22 @@ def test_sessions(self): self.assertCountEqual([str(ses.pk)], (x['id'] for x in d)) q = '?datasets=wheel.position' self.assertFalse(self.ar(self.client.get(reverse('session-list') + q))) + # multiple datasets + q = '?datasets=_ibl_wheel.position.npy,_ibl_trials.table.pqt' + d = self.ar(self.client.get(reverse('session-list') + q)) + self.assertCountEqual([str(ses.pk)], (x['id'] for x in d)) + # datasets + qc (expect to return sessions where defined datasets have correct QC) + q = '?datasets=_ibl_wheel.position.npy,_ibl_trials.table.pqt&dataset_qc_lte=WARNING' + self.assertFalse(self.ar(self.client.get(reverse('session-list') + q))) + q = '?datasets=_ibl_wheel.position.npy&dataset_qc_lte=WARNING' + d = self.ar(self.client.get(reverse('session-list') + q)) + self.assertCountEqual([str(ses.pk)], (x['id'] for x in d), 'failed to return session') + # qc alone (expect to return sessions where any dataset has correct QC) + q = '?dataset_qc_lte=WARNING' + d = self.ar(self.client.get(reverse('session-list') + q)) + self.assertCountEqual([str(ses.pk)], (x['id'] for x in d), 'failed to return session') + q = '?dataset_qc_lte=10' + self.assertFalse(self.ar(self.client.get(reverse('session-list') + q))) def test_surgeries(self): from actions.models import Surgery diff --git a/alyx/actions/views.py b/alyx/actions/views.py index d0b788b0..664c30c0 100644 --- a/alyx/actions/views.py +++ b/alyx/actions/views.py @@ -1,6 +1,7 @@ from datetime import timedelta, date from operator import itemgetter +from one.alf.spec import QC from django.contrib.postgres.fields import JSONField from django.db.models import Count, Q, F, ExpressionWrapper, FloatField from django.db.models.deletion import Collector @@ -226,6 +227,7 @@ class SessionFilter(BaseActionFilter): dataset_types = django_filters.CharFilter(field_name='dataset_types', method='filter_dataset_types') datasets = django_filters.CharFilter(field_name='datasets', method='filter_datasets') + dataset_qc_lte = django_filters.CharFilter(field_name='dataset_qc', method='filter_dataset_qc_lte') performance_gte = django_filters.NumberFilter(field_name='performance', method='filter_performance_gte') performance_lte = django_filters.NumberFilter(field_name='performance', @@ -284,13 +286,23 @@ def filter_dataset_types(self, queryset, _, value): def filter_datasets(self, queryset, _, value): # Note this may later be modified to include collections, e.g. ?datasets=alf/obj.attr.ext + qc = QC.validate(self.request.query_params.get('dataset_qc_lte', QC.FAIL)) dsets = value.split(',') - queryset = queryset.filter(data_dataset_session_related__name__in=dsets) + queryset = queryset.filter(data_dataset_session_related__name__in=dsets, + data_dataset_session_related__qc__lte=qc) queryset = queryset.annotate( dsets_count=Count('data_dataset_session_related', distinct=True)) queryset = queryset.filter(dsets_count__gte=len(dsets)) return queryset + def filter_dataset_qc_lte(self, queryset, _, value): + # If filtering on datasets too, `filter_datasets` handles both QC and Datasets + if 'datasets' in self.request.query_params: + return queryset + qc = QC.validate(value) + queryset = queryset.filter(data_dataset_session_related__qc__lte=qc) + return queryset + def filter_performance_gte(self, queryset, name, perf): queryset = queryset.exclude(n_trials__isnull=True) pf = ExpressionWrapper(100 * F('n_correct_trials') / F('n_trials'), @@ -326,6 +338,8 @@ class SessionAPIList(generics.ListCreateAPIView): - **subject**: subject nickname `/sessions?subject=Algernon` - **dataset_types**: dataset type(s) `/sessions?dataset_types=trials.table,camera.times` - **datasets**: dataset name(s) `/sessions?datasets=_ibl_leftCamera.times.npy` + - **dataset_qc_lte**: dataset QC values less than or equal to this + `/sessions?dataset_qc_lte=WARNING` - **number**: session number - **users**: experimenters (exact) - **date_range**: date `/sessions?date_range=2020-01-12,2020-01-16` @@ -354,9 +368,9 @@ class SessionAPIList(generics.ListCreateAPIView): - **histology**: returns sessions for which the subject has an histology session: `/sessions?histology=True` - **django**: generic filter allowing lookups (same syntax as json filter) - `/sessions?django=project__name__icontains,matlab + `/sessions?django=project__name__icontains,matlab` filters sessions that have matlab in the project name - `/sessions?django=~project__name__icontains,matlab + `/sessions?django=~project__name__icontains,matlab` does the exclusive set: filters sessions that do not have matlab in the project name [===> session model reference](/admin/doc/models/actions.session) diff --git a/alyx/alyx/__init__.py b/alyx/alyx/__init__.py index 37c1153c..04af162b 100644 --- a/alyx/alyx/__init__.py +++ b/alyx/alyx/__init__.py @@ -1 +1 @@ -VERSION = __version__ = '1.18.2' +VERSION = __version__ = '2.0.0' diff --git a/alyx/data/admin.py b/alyx/data/admin.py index f53a34b5..6c3de048 100644 --- a/alyx/data/admin.py +++ b/alyx/data/admin.py @@ -1,7 +1,7 @@ from django.db.models import Count, ProtectedError from django.contrib import admin, messages from django.utils.html import format_html -from django_admin_listfilter_dropdown.filters import RelatedDropdownFilter +from django_admin_listfilter_dropdown.filters import RelatedDropdownFilter, ChoiceDropdownFilter from rangefilter.filters import DateRangeFilter from .models import (DataRepositoryType, DataRepository, DataFormat, DatasetType, @@ -84,16 +84,17 @@ class FileRecordInline(BaseInlineAdmin): class DatasetAdmin(BaseExperimentalDataAdmin): fields = ['name', '_online', 'version', 'dataset_type', 'file_size', 'hash', 'session_ro', 'collection', 'auto_datetime', 'revision_', 'default_dataset', - '_protected', '_public', 'tags'] + '_protected', '_public', 'tags', 'qc'] readonly_fields = ['name_', 'session_ro', '_online', 'auto_datetime', 'revision_', '_protected', '_public', 'tags'] list_display = ['name_', '_online', 'version', 'collection', 'dataset_type_', 'file_size', - 'session_ro', 'created_by', 'created_datetime'] + 'session_ro', 'created_by', 'created_datetime', 'qc'] inlines = [FileRecordInline] list_filter = [('created_by', RelatedDropdownFilter), ('created_datetime', DateRangeFilter), ('dataset_type', RelatedDropdownFilter), - ('tags', RelatedDropdownFilter) + ('tags', RelatedDropdownFilter), + ('qc', ChoiceDropdownFilter) ] search_fields = ('session__id', 'name', 'collection', 'dataset_type__name', 'dataset_type__filename_pattern', 'version') diff --git a/alyx/data/migrations/0019_dataset_qc.py b/alyx/data/migrations/0019_dataset_qc.py new file mode 100644 index 00000000..e2d0cc5d --- /dev/null +++ b/alyx/data/migrations/0019_dataset_qc.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.10 on 2024-02-13 15:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('data', '0018_alter_dataset_collection_alter_revision_name'), + ] + + operations = [ + migrations.AddField( + model_name='dataset', + name='qc', + field=models.IntegerField(choices=[(50, 'CRITICAL'), (40, 'FAIL'), (30, 'WARNING'), (0, 'NOT_SET'), (10, 'PASS')], default=0, help_text='50: CRITICAL / 40: FAIL / 30: WARNING / 0: NOT_SET / 10: PASS'), + ), + ] diff --git a/alyx/data/models.py b/alyx/data/models.py index 83fd08e7..df205f05 100644 --- a/alyx/data/models.py +++ b/alyx/data/models.py @@ -1,4 +1,5 @@ import structlog +from one.alf.spec import QC from django.core.validators import RegexValidator from django.db import models @@ -351,6 +352,10 @@ class Dataset(BaseExperimentalData): help_text="Whether this dataset is the default " "latest revision") + QC_CHOICES = [(e.value, e.name) for e in QC] + qc = models.IntegerField(default=QC.NOT_SET, choices=QC_CHOICES, + help_text=' / '.join([str(q[0]) + ': ' + q[1] for q in QC_CHOICES])) + @property def is_online(self): fr = self.file_records.filter(data_repository__globus_is_personal=False) diff --git a/alyx/data/serializers.py b/alyx/data/serializers.py index 54a131bc..b7858bee 100644 --- a/alyx/data/serializers.py +++ b/alyx/data/serializers.py @@ -5,6 +5,7 @@ from .models import (DataRepositoryType, DataRepository, DataFormat, DatasetType, Dataset, Download, FileRecord, Revision, Tag) from .transfers import _get_session, _change_default_dataset +from alyx.base import BaseSerializerEnumField from actions.models import Session from subjects.models import Subject from misc.models import LabMember @@ -142,6 +143,7 @@ class DatasetSerializer(serializers.HyperlinkedModelSerializer): default_dataset = serializers.BooleanField(required=False, allow_null=True) public = serializers.ReadOnlyField() protected = serializers.ReadOnlyField() + qc = BaseSerializerEnumField(required=False) file_records = DatasetFileRecordsSerializer(read_only=True, many=True) experiment_number = serializers.SerializerMethodField() @@ -213,7 +215,7 @@ class Meta: 'session', 'file_size', 'hash', 'version', 'experiment_number', 'file_records', 'subject', 'date', 'number', 'auto_datetime', 'revision', - 'default_dataset', 'protected', 'public', 'tags') + 'default_dataset', 'protected', 'public', 'tags', 'qc') extra_kwargs = { 'subject': {'write_only': True}, 'date': {'write_only': True}, diff --git a/alyx/data/views.py b/alyx/data/views.py index ff86ffea..316e3eaa 100644 --- a/alyx/data/views.py +++ b/alyx/data/views.py @@ -157,6 +157,7 @@ class DatasetFilter(BaseFilterSet): protected = django_filters.BooleanFilter(method='filter_protected') tag = django_filters.CharFilter('tags__name') revision = django_filters.CharFilter('revision__name') + qc = django_filters.CharFilter(method='enum_field_filter') class Meta: model = Dataset @@ -212,6 +213,7 @@ class DatasetList(generics.ListCreateAPIView): - **tag**: tag name '/datasets?tag=repeated_site - **public**: only returns datasets that are public or not public - **protected**: only returns datasets that are protected or not protected + - **qc**: only returns datasets with this QC value `/datasets?qc=PASS` [===> dataset model reference](/admin/doc/models/data.dataset) """ diff --git a/alyx/experiments/serializers.py b/alyx/experiments/serializers.py index 6685dc5f..0c2c64ec 100644 --- a/alyx/experiments/serializers.py +++ b/alyx/experiments/serializers.py @@ -87,7 +87,7 @@ class Meta: list_serializer_class = FilterDatasetSerializer model = Dataset fields = ('id', 'name', 'dataset_type', 'data_url', 'url', 'file_size', - 'hash', 'version', 'collection') + 'hash', 'version', 'collection', 'qc') class ChronicProbeInsertionListSerializer(serializers.ModelSerializer): diff --git a/alyx/experiments/tests_rest.py b/alyx/experiments/tests_rest.py index 3a2c99cd..a717d86f 100644 --- a/alyx/experiments/tests_rest.py +++ b/alyx/experiments/tests_rest.py @@ -344,9 +344,9 @@ def test_dataset_filters(self): tag, _ = Tag.objects.get_or_create(name='tag_test') d1 = Dataset.objects.create(session=self.session, name='spikes.times.npy', - dataset_type=dtype1, collection='alf/probe_00') + dataset_type=dtype1, collection='alf/probe_00', qc=30) Dataset.objects.create(session=self.session, name='clusters.amps.npy', - dataset_type=dtype2, collection='alf/probe_00') + dataset_type=dtype2, collection='alf/probe_00', qc=40) d1.tags.add(tag) d1.save() @@ -368,10 +368,28 @@ def test_dataset_filters(self): d = self.ar(self.client.get(reverse('probeinsertion-list') + q)) self.assertEqual(len(d), 1) self.assertEqual(probe['id'], d[0]['id']) - q = '?datasets=clusters.amps' self.assertFalse(self.ar(self.client.get(reverse('probeinsertion-list') + q))) + # test dataset + qc filters + q = '?datasets=spikes.times.npy,clusters.amps.npy&dataset_qc_lte=FAIL' + d = self.ar(self.client.get(reverse('probeinsertion-list') + q)) + self.assertEqual(len(d), 1, 'Expect insertion returned as all dsets match QC') + q = '?datasets=spikes.times.npy,clusters.amps.npy&dataset_qc_lte=WARNING' + d = self.ar(self.client.get(reverse('probeinsertion-list') + q)) + self.assertEqual(len(d), 0, 'Expect none returned as one dset doesn''t match QC') + q = '?datasets=spikes.times.npy&dataset_qc_lte=30' # QC code should also work + d = self.ar(self.client.get(reverse('probeinsertion-list') + q)) + self.assertEqual(len(d), 1, 'Expect insertion returned as searched dset matches QC') + + # test qc alone + q = '?dataset_qc_lte=WARNING' + d = self.ar(self.client.get(reverse('probeinsertion-list') + q)) + self.assertEqual(len(d), 1, 'Expect insertion returned as at least 1 dset matches QC') + q = '?dataset_qc_lte=10' # PASS + d = self.ar(self.client.get(reverse('probeinsertion-list') + q)) + self.assertEqual(len(d), 0, 'Expect none returned as no dset matches QC') + # test filtering by tag q = '?tag=tag_test' d = self.ar(self.client.get(reverse('probeinsertion-list') + q)) diff --git a/alyx/experiments/views.py b/alyx/experiments/views.py index db693774..7b329f93 100644 --- a/alyx/experiments/views.py +++ b/alyx/experiments/views.py @@ -1,5 +1,6 @@ import logging +from one.alf.spec import QC from rest_framework import generics from django_filters.rest_framework import CharFilter, UUIDFilter, NumberFilter from django.db.models import Count, Q @@ -73,6 +74,7 @@ class ProbeInsertionFilter(BaseFilterSet): model = CharFilter('model__name') dataset_types = CharFilter(field_name='dataset_types', method='filter_dataset_types') datasets = CharFilter(field_name='datasets', method='filter_datasets') + dataset_qc_lte = CharFilter(field_name='dataset_qc', method='filter_dataset_qc_lte') lab = CharFilter(field_name='session__lab__name', lookup_expr='iexact') project = CharFilter(field_name='session__project__name', lookup_expr='icontains') task_protocol = CharFilter(field_name='session__task_protocol', lookup_expr='icontains') @@ -110,13 +112,21 @@ def filter_dataset_types(self, queryset, _, value): return queryset def filter_datasets(self, queryset, _, value): + qc = QC.validate(self.request.query_params.get('dataset_qc_lte', QC.FAIL)) dsets = value.split(',') - queryset = queryset.filter(datasets__name__in=dsets) + queryset = queryset.filter(datasets__name__in=dsets, datasets__qc__lte=qc) queryset = queryset.annotate( dsets_count=Count('datasets', distinct=True)) queryset = queryset.filter(dsets_count__gte=len(dsets)) return queryset + def filter_dataset_qc_lte(self, queryset, _, value): + # If filtering on datasets too, `filter_datasets` handles both QC and Datasets + if 'datasets' in self.request.query_params: + return queryset + qc = QC.validate(value) + return queryset.filter(datasets__qc__lte=qc) + class Meta: model = ProbeInsertion exclude = ['json'] @@ -139,6 +149,7 @@ class ProbeInsertionList(generics.ListCreateAPIView): - **tag**: tag name (icontains) - **dataset_types**: dataset type(s) - **datasets**: datasets name(s) + - **dataset_qc_lte**: dataset QC value, e.g. PASS, WARNING, FAIL, CRITICAL - **atlas_name**: returns a session if any channel name icontains the value: `/insertions?brain_region=visual cortex` - **atlas_acronym**: returns a session if any of its channels name exactly diff --git a/alyx/misc/management/commands/one_cache.py b/alyx/misc/management/commands/one_cache.py index 0ec25dbe..a5e96ec9 100644 --- a/alyx/misc/management/commands/one_cache.py +++ b/alyx/misc/management/commands/one_cache.py @@ -16,6 +16,8 @@ import pyarrow as pa from tqdm import tqdm from one.alf.cache import _metadata +from one.util import QC_TYPE +from one.alf.spec import QC from one.remote.aws import get_s3_virtual_host from django.db import connection @@ -30,7 +32,7 @@ from experiments.models import ProbeInsertion logger = logging.getLogger(__name__) -ONE_API_VERSION = '1.13.0' # Minimum compatible ONE api version +ONE_API_VERSION = '2.7.0' # Minimum compatible ONE api version def measure_time(func): @@ -382,7 +384,7 @@ def generate_datasets_frame(tags=None, batch_size=100_000) -> pd.DataFrame: fields = ( 'id', 'name', 'file_size', 'hash', 'collection', 'revision__name', 'default_dataset', 'session__id', 'session__start_time__date', 'session__number', - 'session__subject__nickname', 'session__lab__name', 'exists_flatiron', 'exists_aws' + 'session__subject__nickname', 'session__lab__name', 'exists_flatiron', 'exists_aws', 'qc' ) fields_map = {'session__id': 'eid', 'default_dataset': 'default_revision'} @@ -411,6 +413,9 @@ def generate_datasets_frame(tags=None, batch_size=100_000) -> pd.DataFrame: df[['id', 'eid']] = df[['id', 'eid']].astype(str) df = df.set_index(['eid', 'id']) + # Convert QC enum int to pandas category + df['qc'] = pd.Categorical([QC(i).name for i in df['qc']], dtype=QC_TYPE) + all_df = pd.concat([all_df, df], ignore_index=False, copy=False) logger.debug(f'Final datasets frame = {getsizeof(all_df) / 1024 ** 2:.1f} MiB') diff --git a/requirements.txt b/requirements.txt index 61b645d4..5463f901 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,4 +28,4 @@ python-magic pytz structlog>=21.5.0 webdavclient3 -ONE-api>=2.1.0 +ONE-api>=2.7.0 From d55f2b394de190a5601a90f54b4085dfdd54afe6 Mon Sep 17 00:00:00 2001 From: Miles Wells Date: Fri, 1 Mar 2024 15:58:44 +0000 Subject: [PATCH 3/9] ONE prerelease --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 5463f901..d4d94885 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,4 +28,4 @@ python-magic pytz structlog>=21.5.0 webdavclient3 -ONE-api>=2.7.0 +ONE-api>=2.7rc From 03c959cb8379239836174d81075ac17b7c68e248 Mon Sep 17 00:00:00 2001 From: Miles Wells Date: Fri, 1 Mar 2024 16:13:34 +0000 Subject: [PATCH 4/9] flake --- alyx/actions/tests_rest.py | 6 ++++-- alyx/actions/views.py | 7 ++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/alyx/actions/tests_rest.py b/alyx/actions/tests_rest.py index 8bf83c2a..27531128 100644 --- a/alyx/actions/tests_rest.py +++ b/alyx/actions/tests_rest.py @@ -266,8 +266,10 @@ def test_sessions(self): # test dataset type filters dtype1, _ = DatasetType.objects.get_or_create(name='trials.table') dtype2, _ = DatasetType.objects.get_or_create(name='wheel.position') - Dataset.objects.create(session=ses, name='_ibl_trials.table.pqt', dataset_type=dtype1, qc=40) - Dataset.objects.create(session=ses, name='_ibl_wheel.position.npy', dataset_type=dtype2, qc=30) + Dataset.objects.create( + session=ses, name='_ibl_trials.table.pqt', dataset_type=dtype1, qc=40) + Dataset.objects.create( + session=ses, name='_ibl_wheel.position.npy', dataset_type=dtype2, qc=30) d = self.ar(self.client.get(reverse('session-list') + '?dataset_types=wheel.position')) self.assertCountEqual([str(ses.pk)], (x['id'] for x in d)) q = '?dataset_types=wheel.position,trials.table' # Check with list diff --git a/alyx/actions/views.py b/alyx/actions/views.py index 664c30c0..d9df6054 100644 --- a/alyx/actions/views.py +++ b/alyx/actions/views.py @@ -224,10 +224,11 @@ class ProcedureTypeList(generics.ListCreateAPIView): class SessionFilter(BaseActionFilter): - dataset_types = django_filters.CharFilter(field_name='dataset_types', - method='filter_dataset_types') + dataset_types = django_filters.CharFilter( + field_name='dataset_types', method='filter_dataset_types') datasets = django_filters.CharFilter(field_name='datasets', method='filter_datasets') - dataset_qc_lte = django_filters.CharFilter(field_name='dataset_qc', method='filter_dataset_qc_lte') + dataset_qc_lte = django_filters.CharFilter( + field_name='dataset_qc', method='filter_dataset_qc_lte') performance_gte = django_filters.NumberFilter(field_name='performance', method='filter_performance_gte') performance_lte = django_filters.NumberFilter(field_name='performance', From b8820b31e0e870c022c2a0e2271c55f573573757 Mon Sep 17 00:00:00 2001 From: Miles Wells Date: Tue, 5 Mar 2024 16:01:11 -0500 Subject: [PATCH 5/9] Model help string typo; more field validation --- alyx/actions/models.py | 2 +- alyx/data/admin.py | 2 +- alyx/data/serializers.py | 5 +++++ alyx/data/tests_rest.py | 3 +++ alyx/data/transfers.py | 10 +++++++++- alyx/data/views.py | 15 ++++++++++++--- alyx/misc/management/commands/one_cache.py | 2 +- 7 files changed, 32 insertions(+), 7 deletions(-) diff --git a/alyx/actions/models.py b/alyx/actions/models.py index edec7c74..fd706e3a 100644 --- a/alyx/actions/models.py +++ b/alyx/actions/models.py @@ -260,7 +260,7 @@ class Session(BaseAction): help_text=' / '.join([str(q[0]) + ': ' + q[1] for q in QC_CHOICES])) extended_qc = models.JSONField(null=True, blank=True, - help_text="Structured data about session QC," + help_text="Structured data about session QC, " "formatted in a user-defined way") auto_datetime = models.DateTimeField(auto_now=True, blank=True, null=True, diff --git a/alyx/data/admin.py b/alyx/data/admin.py index 6c3de048..f8d503f1 100644 --- a/alyx/data/admin.py +++ b/alyx/data/admin.py @@ -86,7 +86,7 @@ class DatasetAdmin(BaseExperimentalDataAdmin): 'session_ro', 'collection', 'auto_datetime', 'revision_', 'default_dataset', '_protected', '_public', 'tags', 'qc'] readonly_fields = ['name_', 'session_ro', '_online', 'auto_datetime', 'revision_', - '_protected', '_public', 'tags'] + '_protected', '_public', 'tags', 'qc'] list_display = ['name_', '_online', 'version', 'collection', 'dataset_type_', 'file_size', 'session_ro', 'created_by', 'created_datetime', 'qc'] inlines = [FileRecordInline] diff --git a/alyx/data/serializers.py b/alyx/data/serializers.py index b7858bee..a10f02f5 100644 --- a/alyx/data/serializers.py +++ b/alyx/data/serializers.py @@ -2,6 +2,8 @@ from rest_framework import serializers from django.db.models import Count, Q, BooleanField +from one.alf.spec import QC + from .models import (DataRepositoryType, DataRepository, DataFormat, DatasetType, Dataset, Download, FileRecord, Revision, Tag) from .transfers import _get_session, _change_default_dataset @@ -180,6 +182,9 @@ def create(self, validated_data): name = validated_data.get('name', None) default = validated_data.get('default_dataset', None) session = validated_data.get('session', None) + # validate QC value + if 'qc' in validated_data: + validated_data['qc'] = QC.validate(validated_data['qc']) if session: if default is not False: diff --git a/alyx/data/tests_rest.py b/alyx/data/tests_rest.py index 6ffbb410..8a861243 100644 --- a/alyx/data/tests_rest.py +++ b/alyx/data/tests_rest.py @@ -145,6 +145,8 @@ def test_dataset(self): self.assertEqual(r.data['collection'], None) # Check that it has been set as the default dataset self.assertEqual(r.data['default_dataset'], True) + # Check QC value is NOT_SET by default + self.assertEqual(r.data['qc'], 'NOT_SET') # Make sure a session has been created. session = r.data['session'] r = self.client.get(session) @@ -162,6 +164,7 @@ def test_dataset(self): 'date': '2018-01-01', 'number': 2, 'collection': 'test_path', + 'qc': 'PASS' } r = self.post(reverse('dataset-list'), data) diff --git a/alyx/data/transfers.py b/alyx/data/transfers.py index 01c219b3..3cc730ab 100644 --- a/alyx/data/transfers.py +++ b/alyx/data/transfers.py @@ -11,6 +11,8 @@ import numpy as np from one.alf.files import add_uuid_string, folder_parts from one.registration import get_dataset_type +from one.util import ensure_list +from one.alf.spec import QC from alyx import settings from data.models import FileRecord, Dataset, DatasetType, DataFormat, DataRepository @@ -244,7 +246,7 @@ def _check_dataset_protected(session, collection, filename): def _create_dataset_file_records( rel_dir_path=None, filename=None, session=None, user=None, repositories=None, exists_in=None, collection=None, hash=None, - file_size=None, version=None, revision=None, default=None): + file_size=None, version=None, revision=None, default=None, qc=None): assert session is not None revision_name = f'#{revision.name}#' if revision else '' @@ -265,6 +267,12 @@ def _create_dataset_file_records( dataset_type=dataset_type, data_format=data_format, revision=revision ) dataset.default_dataset = default is True + try: + dataset.qc = int(QC.validate(qc or 'NOT_SET')) + except ValueError: + data = {'status_code': 400, + 'detail': f'Invalid QC value "{qc}" for dataset "{relative_path}"'} + return None, Response(data=data, status=403) dataset.save() # If the dataset already existed see if it is protected (i.e can't be overwritten) diff --git a/alyx/data/views.py b/alyx/data/views.py index 316e3eaa..ce76021f 100644 --- a/alyx/data/views.py +++ b/alyx/data/views.py @@ -308,6 +308,7 @@ def _make_dataset_response(dataset): 'collection': dataset.collection, 'revision': getattr(dataset.revision, 'name', None), 'default': dataset.default_dataset, + 'qc': dataset.qc } out['file_records'] = file_records return out @@ -355,6 +356,7 @@ def create(self, request): 'hashes': ['f9c26e42-8f22-4f07-8fdd-bb51a63bedaa', 'f9c26e42-8f22-4f07-8fdd-bb51a63bedad'] # optional 'filesizes': [145684, 354213], # optional + 'qc': ['NOT_SET', 'PASS'], # optional 'server_only': True, # optional, defaults to False. Will only create file # records in the server repositories and skips local repositories 'versions': ['1.4.4', '1.4.4'], # optional, usually refers to the software version @@ -375,7 +377,7 @@ def create(self, request): ``` If the dataset already exists, it will use the file hash to deduce if the file has been - patched or not (ie. the filerecords will be created as not existing) + patched or not (i.e. the filerecords will be created as not existing) """ user = request.data.get('created_by', None) if user: @@ -422,6 +424,13 @@ def create(self, request): if isinstance(filesizes, str): filesizes = filesizes.split(',') + # qc if provided + qcs = request.data.get('qc', [None] * len(filenames)) or 'NOT_SET' + if isinstance(qcs, str): + qcs = qcs.split(',') + if len(qcs) == 1: + qcs = qcs * len(filenames) + # flag to discard file records creation on local repositories, defaults to False server_only = request.data.get('server_only', False) if isinstance(server_only, str): @@ -482,7 +491,7 @@ def create(self, request): return Response(data=data, status=403) response = [] - for filename, hash, fsize, version in zip(filenames, hashes, filesizes, versions): + for filename, hash, fsize, version, qc in zip(filenames, hashes, filesizes, versions, qcs): if not filename: continue info, resp = _get_name_collection_revision(filename, rel_dir_path) @@ -499,7 +508,7 @@ def create(self, request): collection=info['collection'], rel_dir_path=info['rel_dir_path'], filename=info['filename'], session=session, user=user, repositories=repositories, exists_in=exists_in, hash=hash, file_size=fsize, version=version, - revision=revision, default=default) + revision=revision, default=default, qc=qc) if resp: return resp out = _make_dataset_response(dataset) diff --git a/alyx/misc/management/commands/one_cache.py b/alyx/misc/management/commands/one_cache.py index a5e96ec9..7c3ea682 100644 --- a/alyx/misc/management/commands/one_cache.py +++ b/alyx/misc/management/commands/one_cache.py @@ -32,7 +32,7 @@ from experiments.models import ProbeInsertion logger = logging.getLogger(__name__) -ONE_API_VERSION = '2.7.0' # Minimum compatible ONE api version +ONE_API_VERSION = '2.7' # Minimum compatible ONE api version def measure_time(func): From e7667ed0a1c3a2b7dbc313782a89b6fb14fd815a Mon Sep 17 00:00:00 2001 From: Miles Wells Date: Wed, 6 Mar 2024 11:18:39 -0500 Subject: [PATCH 6/9] laserStimulation.intervals dataset type fixture --- alyx/data/fixtures/data.datasettype.json | 11 +++++++++++ requirements.txt | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/alyx/data/fixtures/data.datasettype.json b/alyx/data/fixtures/data.datasettype.json index 910d5550..582c9dc2 100644 --- a/alyx/data/fixtures/data.datasettype.json +++ b/alyx/data/fixtures/data.datasettype.json @@ -2220,5 +2220,16 @@ "description": "Look up table from photometry ROI, to fiber name registered in the database and Allen brain location", "filename_pattern": "*photometryROI.locations*" } + }, + { + "model": "data.datasettype", + "pk": "140cd2a9-91c1-45ee-9d19-77e8d39abb5f", + "fields": { + "json": null, + "name": "laserStimulation.intervals", + "created_by": null, + "description": "The start and end times of the laser stimulation period.", + "filename_pattern": "" + } } ] diff --git a/requirements.txt b/requirements.txt index d4d94885..cdf85406 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,4 +28,4 @@ python-magic pytz structlog>=21.5.0 webdavclient3 -ONE-api>=2.7rc +ONE-api~=2.7rc0 From 2c5477f732180dfa00ef29eacafab3a430257178 Mon Sep 17 00:00:00 2001 From: Miles Wells Date: Tue, 12 Mar 2024 15:22:38 +0200 Subject: [PATCH 7/9] Expose dataset JSON field --- alyx/data/serializers.py | 2 +- alyx/data/transfers.py | 1 - setup.cfg | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/alyx/data/serializers.py b/alyx/data/serializers.py index a10f02f5..c455611f 100644 --- a/alyx/data/serializers.py +++ b/alyx/data/serializers.py @@ -220,7 +220,7 @@ class Meta: 'session', 'file_size', 'hash', 'version', 'experiment_number', 'file_records', 'subject', 'date', 'number', 'auto_datetime', 'revision', - 'default_dataset', 'protected', 'public', 'tags', 'qc') + 'default_dataset', 'protected', 'public', 'tags', 'qc', 'json') extra_kwargs = { 'subject': {'write_only': True}, 'date': {'write_only': True}, diff --git a/alyx/data/transfers.py b/alyx/data/transfers.py index 3cc730ab..bc6b744e 100644 --- a/alyx/data/transfers.py +++ b/alyx/data/transfers.py @@ -11,7 +11,6 @@ import numpy as np from one.alf.files import add_uuid_string, folder_parts from one.registration import get_dataset_type -from one.util import ensure_list from one.alf.spec import QC from alyx import settings diff --git a/setup.cfg b/setup.cfg index ca0902f7..d332421a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -4,7 +4,7 @@ universal = 1 [tool:pytest] [flake8] -ignore = E117,E265,E731,F403,E741,E722,W504 +ignore = E117,E265,E731,F403,E741,E722,W504,D max-line-length = 99 exclude = migrations From aba9e1be5cc1b6df0ef2c14731f8eb704312df86 Mon Sep 17 00:00:00 2001 From: k1o0 Date: Wed, 13 Mar 2024 18:55:24 +0200 Subject: [PATCH 8/9] Fix typo in session extended_qc field description --- .../0021_alter_session_extended_qc.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 alyx/actions/migrations/0021_alter_session_extended_qc.py diff --git a/alyx/actions/migrations/0021_alter_session_extended_qc.py b/alyx/actions/migrations/0021_alter_session_extended_qc.py new file mode 100644 index 00000000..17a3e557 --- /dev/null +++ b/alyx/actions/migrations/0021_alter_session_extended_qc.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.10 on 2024-03-12 13:55 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('actions', '0020_alter_notification_notification_type_and_more'), + ] + + operations = [ + migrations.AlterField( + model_name='session', + name='extended_qc', + field=models.JSONField(blank=True, help_text='Structured data about session QC, formatted in a user-defined way', null=True), + ), + ] From 8c067a801ff198c48869990a02dc4b250a5b0747 Mon Sep 17 00:00:00 2001 From: k1o0 Date: Thu, 14 Mar 2024 15:00:43 +0200 Subject: [PATCH 9/9] Test register-files QC validation; improve setup.py get user --- alyx/data/tests_rest.py | 20 ++++++++++++++++++++ setup.py | 6 +++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/alyx/data/tests_rest.py b/alyx/data/tests_rest.py index 8a861243..305de8ae 100644 --- a/alyx/data/tests_rest.py +++ b/alyx/data/tests_rest.py @@ -128,6 +128,7 @@ def test_dataset_filerecord(self): self.assertTrue(new_mod_date > mod_date) def test_dataset(self): + # Test dataset creation via the datasets endpoint data = { 'name': 'some-dataset', 'dataset_type': 'dst', @@ -172,6 +173,7 @@ def test_dataset(self): self.assertEqual(r.data['revision'], None) self.assertEqual(r.data['collection'], data['collection']) self.assertEqual(r.data['default_dataset'], True) + self.assertEqual(r.data['qc'], 'PASS') data_url = r.data['url'] # But if we change the collection, we are okay @@ -345,6 +347,24 @@ def test_register_files_hostname(self): self.assertEqual(ds0.version, '1.1.1') self.assertEqual(ds1.version, '2.2.2') + def test_qc_validation(self): + # this tests the validation of dataset QC outcomes + data = { + 'path': '%s/2018-01-01/2/dir' % self.subject, + 'filenames': 'a.b.e1,a.c.e2', + 'hostname': 'hostname', + 'qc': '10,critical' # Both numerical and string QC values should be parsed + } + r = self.post(reverse('register-file'), data) + records = self.ar(r, 201) + self.assertEqual([10, 50], [rec['qc'] for rec in records]) + self._assert_registration(r, data) + # a single QC value should be applied to all datasets + data['qc'] = 'FAIL' + r = self.post(reverse('register-file'), data) + records = self.ar(r, 201) + self.assertEqual([40, 40], [rec['qc'] for rec in records]) + def test_register_files_hash(self): # this is old use case where we register one dataset according to the hostname, no need # for a lab in this case diff --git a/setup.py b/setup.py index 92bb3f3e..ecba39ac 100755 --- a/setup.py +++ b/setup.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 from pathlib import Path -from getpass import getpass +from getpass import getpass, getuser import os import os.path as op import platform @@ -141,8 +141,8 @@ def _replace_in_file(source_file, target_file, replacements=None, target_mode='w try: _system(f'sudo mkdir -p {file_log_json.parent}') _system(f'sudo mkdir -p {file_log.parent}') - _system(f'sudo chown {os.getlogin()}:www-data -fR {file_log.parent}') - _system(f'sudo chown {os.getlogin()}:www-data -fR {file_log_json.parent}') + _system(f'sudo chown {getuser()}:www-data -fR {file_log.parent}') + _system(f'sudo chown {getuser()}:www-data -fR {file_log_json.parent}') _system(f'touch {file_log_json}') _system(f'touch {file_log}') _system('python3 alyx/manage.py makemigrations')