From afad6659621d40cde6137ff87856ae6a2aa771d0 Mon Sep 17 00:00:00 2001 From: Daniel Chiquito Date: Wed, 1 Dec 2021 10:32:00 -0500 Subject: [PATCH 01/10] Add global_import_export flag to Project model --- .../0022_project_global_import_export.py | 18 ++++++++++++++++++ miqa/core/models/project.py | 1 + 2 files changed, 19 insertions(+) create mode 100644 miqa/core/migrations/0022_project_global_import_export.py diff --git a/miqa/core/migrations/0022_project_global_import_export.py b/miqa/core/migrations/0022_project_global_import_export.py new file mode 100644 index 00000000..60cb6be7 --- /dev/null +++ b/miqa/core/migrations/0022_project_global_import_export.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.9 on 2021-11-30 22:28 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0021_update_qc_options'), + ] + + operations = [ + migrations.AddField( + model_name='project', + name='global_import_export', + field=models.BooleanField(default=False), + ), + ] diff --git a/miqa/core/models/project.py b/miqa/core/models/project.py index ce890d39..a952da6f 100644 --- a/miqa/core/models/project.py +++ b/miqa/core/models/project.py @@ -37,6 +37,7 @@ class Project(TimeStampedModel, models.Model): archived = models.BooleanField(default=False) import_path = models.CharField(max_length=500) export_path = models.CharField(max_length=500) + global_import_export = models.BooleanField(default=False) evaluation_models = models.JSONField(default=default_evaluation_model_mapping) def __str__(self): From 03145e0cf2aa05528a3bcb094d815b57766b5020 Mon Sep 17 00:00:00 2001 From: Daniel Chiquito Date: Wed, 1 Dec 2021 10:34:21 -0500 Subject: [PATCH 02/10] Add global import/export logic --- miqa/core/tasks.py | 65 +++++++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/miqa/core/tasks.py b/miqa/core/tasks.py index 90809eb1..7627e823 100644 --- a/miqa/core/tasks.py +++ b/miqa/core/tasks.py @@ -68,11 +68,13 @@ def perform_import(import_dict, project_id): # when multi-project imports are supported project = Project.objects.get(id=project_id) - for _project_name, project_data in import_dict['projects'].items(): - # Switch these lines to support multi-project imports - # project_object = Project(name=_project_name) - # new_projects.append(project_object) - project_object = project + for project_name, project_data in import_dict['projects'].items(): + if project.global_import_export: + # A global import uses the project name column to determine which project to import to + project_object = Project.objects.get(name=project_name) + else: + # A normal import ignores the project name column + project_object = project # delete old imports of these projects Experiment.objects.filter( @@ -107,28 +109,49 @@ def perform_import(import_dict, project_id): def export_data(project_id): - project_object = Project.objects.get(id=project_id) - parent_location = Path(project_object.export_path).parent + project = Project.objects.get(id=project_id) + parent_location = Path(project.export_path).parent if not parent_location.exists(): raise ValueError(f'No such location {parent_location} to create export file.') - perform_export.delay(project_id) + + # In the event of a global export, we only want to export the projects listed in the import + # file. Read the import file now and extract the project names. + if project.import_path.endswith('.csv'): + import_dict = import_dataframe_to_dict(pandas.read_csv(project.import_path)) + elif project.import_path.endswith('.json'): + import_dict = json.load(open(project.import_path)) + else: + raise ValueError(f'Invalid import file {project.import_path}.') + + import_dict = validate_import_dict(import_dict, project) + project_names = import_dict['projects'].keys() + + perform_export.delay(project_id, project_names) @shared_task -def perform_export(project_id): - project_object = Project.objects.get(id=project_id) +def perform_export(project_id, project_names): + project = Project.objects.get(id=project_id) data = [] - for frame_object in Frame.objects.filter(scan__experiment__project=project_object): - data.append( - [ - project_object.name, - frame_object.scan.experiment.name, - frame_object.scan.name, - frame_object.scan.scan_type, - frame_object.frame_number, - frame_object.raw_path, - ] - ) + if project.global_import_export: + # A global export should export all projects listed in the import file + projects = [Project.objects.get(name=project_name) for project_name in project_names] + else: + # A normal export should only export the current project + projects = [project] + + for project_object in projects: + for frame_object in Frame.objects.filter(scan__experiment__project=project_object): + data.append( + [ + project_object.name, + frame_object.scan.experiment.name, + frame_object.scan.name, + frame_object.scan.scan_type, + frame_object.frame_number, + frame_object.raw_path, + ] + ) export_df = pandas.DataFrame(data, columns=IMPORT_CSV_COLUMNS) export_df.to_csv(project_object.export_path, index=False) From 3414e695de358fb924ee7fff11a6f075d617fb26 Mon Sep 17 00:00:00 2001 From: Daniel Chiquito Date: Wed, 1 Dec 2021 10:35:20 -0500 Subject: [PATCH 03/10] Add globalImportExport to settings API --- miqa/core/rest/project.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/miqa/core/rest/project.py b/miqa/core/rest/project.py index 0e4aa91c..abe21617 100644 --- a/miqa/core/rest/project.py +++ b/miqa/core/rest/project.py @@ -16,10 +16,11 @@ class ProjectSettingsSerializer(serializers.ModelSerializer): class Meta: model = Project - fields = ['importPath', 'exportPath', 'permissions'] + fields = ['importPath', 'exportPath', 'globalImportExport', 'permissions'] importPath = serializers.CharField(source='import_path') # noqa: N815 exportPath = serializers.CharField(source='export_path') # noqa: N815 + globalImportExport = serializers.BooleanField(source='global_import_export') # noqa: N815 permissions = serializers.SerializerMethodField('get_permissions') def get_permissions(self, obj): @@ -117,6 +118,7 @@ def settings_(self, request, **kwargs): project.import_path = request.data['importPath'] project.export_path = request.data['exportPath'] + project.global_import_export = request.data['globalImportExport'] project.full_clean() project.save() serializer = ProjectSettingsSerializer(project) From 561f0c52bfeab5dcc7fe61d3b115cd95b8ab09e5 Mon Sep 17 00:00:00 2001 From: Daniel Chiquito Date: Wed, 1 Dec 2021 10:35:50 -0500 Subject: [PATCH 04/10] Validate project names during global import This causes the import API call to fail with a validation error, rather than succeeding and allowing the task to fail later, which does not report anything in the UI. --- miqa/core/conversion/import_export_csvs.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/miqa/core/conversion/import_export_csvs.py b/miqa/core/conversion/import_export_csvs.py index d065d5c2..b38a5da9 100644 --- a/miqa/core/conversion/import_export_csvs.py +++ b/miqa/core/conversion/import_export_csvs.py @@ -2,6 +2,8 @@ from schema import And, Schema, SchemaError, Use +from miqa.core.models import Project + IMPORT_CSV_COLUMNS = [ 'project_name', 'experiment_name', @@ -30,7 +32,7 @@ def validate_file_locations(input_dict, project): return input_dict -def validate_import_dict(import_dict, project): +def validate_import_dict(import_dict, project: Project): import_schema = Schema( { 'projects': { @@ -52,9 +54,13 @@ def validate_import_dict(import_dict, project): try: import_schema.validate(import_dict) import_dict = validate_file_locations(import_dict, project) - return import_dict except SchemaError: raise ValueError(f'Invalid format of import file {project.import_path}') + if project.global_import_export: + for project_name in import_dict['projects']: + if not Project.objects.filter(name=project_name).exists(): + raise ValueError(f'Project {project_name} does not exist') + return import_dict def import_dataframe_to_dict(df): From ddacc7ce417bd5a266a3ad68db8af69f2b659ca6 Mon Sep 17 00:00:00 2001 From: Daniel Chiquito Date: Wed, 1 Dec 2021 10:37:12 -0500 Subject: [PATCH 05/10] Set celery always_eager to True for testing --- miqa/settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/miqa/settings.py b/miqa/settings.py index cecb730c..59b7fc92 100644 --- a/miqa/settings.py +++ b/miqa/settings.py @@ -65,7 +65,8 @@ class DevelopmentConfiguration(MiqaMixin, DevelopmentBaseConfiguration): class TestingConfiguration(MiqaMixin, TestingBaseConfiguration): - pass + # We would like to test that the celery tasks work correctly when triggered from the API + CELERY_TASK_ALWAYS_EAGER = True class ProductionConfiguration(MiqaMixin, ProductionBaseConfiguration): From f041d38c8a5dca93bcc3951e2bd974320c21be75 Mon Sep 17 00:00:00 2001 From: Daniel Chiquito Date: Wed, 1 Dec 2021 10:38:05 -0500 Subject: [PATCH 06/10] Add global_import_export to existing tests --- miqa/core/tests/factories.py | 1 + miqa/core/tests/test_rest.py | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/miqa/core/tests/factories.py b/miqa/core/tests/factories.py index cf4c36b2..59c3bd69 100644 --- a/miqa/core/tests/factories.py +++ b/miqa/core/tests/factories.py @@ -25,6 +25,7 @@ class Meta: import_path = factory.Faker('file_path') export_path = factory.Faker('file_path') + global_import_export = False class ExperimentFactory(factory.django.DjangoModelFactory): diff --git a/miqa/core/tests/test_rest.py b/miqa/core/tests/test_rest.py index ac411510..3c54fad6 100644 --- a/miqa/core/tests/test_rest.py +++ b/miqa/core/tests/test_rest.py @@ -36,8 +36,9 @@ def test_project_settings_get(user_api_client, project, user): assert all(key in resp.data for key in ['importPath', 'exportPath', 'permissions']) +@pytest.mark.parametrize('global_import_export', [True, False]) @pytest.mark.django_db -def test_project_settings_put(user_api_client, project, user): +def test_project_settings_put(user_api_client, project, user, global_import_export): user_api_client = user_api_client() my_perms = get_perms(user, project) new_perms = { @@ -50,6 +51,7 @@ def test_project_settings_put(user_api_client, project, user): data={ 'importPath': '/new/fake/path', 'exportPath': '/new/fake/path', + 'globalImportExport': global_import_export, 'permissions': new_perms, }, ) @@ -71,6 +73,7 @@ def test_project_settings_put(user_api_client, project, user): assert user_api_client.get(f'/api/v1/projects/{project.id}/settings').data == { 'importPath': '/new/fake/path', 'exportPath': '/new/fake/path', + 'globalImportExport': global_import_export, 'permissions': expected_perms, } my_new_perms = get_perms(user, project) @@ -87,6 +90,7 @@ def test_settings_endpoint_requires_superuser(user_api_client, project, user): data={ 'importPath': '/new/fake/path', 'exportPath': '/new/fake/path', + 'globalImportExport': False, 'permissions': {}, }, ) From e72c66f2fb46f247ec1ff282ca4363e2c032ef37 Mon Sep 17 00:00:00 2001 From: Daniel Chiquito Date: Wed, 1 Dec 2021 10:38:44 -0500 Subject: [PATCH 07/10] Generate correct project names in test_import --- miqa/core/tests/test_import.py | 53 +++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/miqa/core/tests/test_import.py b/miqa/core/tests/test_import.py index 7f0ef834..963ee6c3 100644 --- a/miqa/core/tests/test_import.py +++ b/miqa/core/tests/test_import.py @@ -22,10 +22,14 @@ def generate_import_csv(sample_scans): writer = csv.DictWriter(output, fieldnames=fieldnames, dialect='unix') writer.writeheader() for scan_folder, scan_id, scan_type in sample_scans: + # The project name is encoded somewhere in the path + for potential_name in ['ohsu', 'ucsd']: + if potential_name in scan_folder: + project_name = potential_name writer.writerow( { - 'project_name': 'testProject', - 'experiment_name': 'testExperiment', + 'project_name': project_name, + 'experiment_name': f'{scan_id}_experiment', 'scan_name': scan_id, 'scan_type': scan_type, 'frame_number': 0, @@ -37,33 +41,36 @@ def generate_import_csv(sample_scans): def generate_import_json(samples_dir: Path, sample_scans): - return { - 'projects': { - 'testProject': { - 'experiments': { - 'testExperiment': { - 'scans': { - scan_id: { - 'type': scan_type, - 'frames': { - 0: { - 'file_location': str( - Path( - scan_folder, - f'{scan_id}_{scan_type}', - 'image.nii.gz', - ) + projects = {} + for scan_folder, scan_id, scan_type in sample_scans: + # The project name is encoded somewhere in the path + for potential_name in ['ohsu', 'ucsd']: + if potential_name in scan_folder: + project_name = potential_name + experiment_name = f'{scan_id}_experiment' + projects[project_name] = { + 'experiments': { + experiment_name: { + 'scans': { + scan_id: { + 'type': scan_type, + 'frames': { + 0: { + 'file_location': str( + Path( + scan_folder, + f'{scan_id}_{scan_type}', + 'image.nii.gz', ) - } - }, - } - for scan_folder, scan_id, scan_type in sample_scans + ) + } + }, } } } } } - } + return {'projects': projects} @pytest.mark.django_db From 7ad7888879316c0d681e4ee5d7528a2ed3588d1b Mon Sep 17 00:00:00 2001 From: Daniel Chiquito Date: Wed, 1 Dec 2021 10:38:57 -0500 Subject: [PATCH 08/10] Add tests for global import --- miqa/core/tests/test_import.py | 83 ++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/miqa/core/tests/test_import.py b/miqa/core/tests/test_import.py index 963ee6c3..7d77245c 100644 --- a/miqa/core/tests/test_import.py +++ b/miqa/core/tests/test_import.py @@ -80,12 +80,51 @@ def test_import_csv(tmp_path, user, project_factory, sample_scans, user_api_clie output, _writer = generate_import_csv(sample_scans) fd.write(output.getvalue()) + # The default project will have a name which does not match the import CSV column project = project_factory(import_path=csv_file) user_api_client = user_api_client(project=project) resp = user_api_client.post(f'/api/v1/projects/{project.id}/import') if user.is_superuser: assert resp.status_code == 204 + # The import should update the project, even though the names do not match + project.refresh_from_db() + assert project.experiments.count() == 2 + assert project.experiments.all()[0].scans.count() == 1 + assert project.experiments.all()[1].scans.count() == 1 + else: + assert resp.status_code == 401 + + +@pytest.mark.django_db(transaction=True) +def test_import_global_csv(tmp_path, user, project_factory, sample_scans, user_api_client): + # Generate an import CSV for two projects with the same data + csv_file = str(tmp_path / 'import.csv') + with open(csv_file, 'w') as fd: + output, _writer = generate_import_csv(sample_scans) + fd.write(output.getvalue()) + + # noqa:E501 Create a project with the global_import_export flag enabled, but which does not match the project names + project = project_factory( + import_path=csv_file, name='projectForImporting', global_import_export=True + ) + # Create projects targeted by the import + project_ohsu = project_factory(import_path=csv_file, name='ohsu') + project_ucsd = project_factory(import_path=csv_file, name='ucsd') + user_api_client = user_api_client(project=project) + + resp = user_api_client.post(f'/api/v1/projects/{project.id}/import') + if user.is_superuser: + assert resp.status_code == 204 + # The import should update the correctly named projects, but not the original import project + project.refresh_from_db() + project_ohsu.refresh_from_db() + project_ucsd.refresh_from_db() + assert project.experiments.count() == 0 + assert project_ohsu.experiments.count() == 1 + assert project_ohsu.experiments.get().scans.count() == 1 + assert project_ucsd.experiments.count() == 1 + assert project_ucsd.experiments.get().scans.count() == 1 else: assert resp.status_code == 401 @@ -103,11 +142,55 @@ def test_import_json( with open(json_file, 'w') as fd: fd.write(json.dumps(generate_import_json(samples_dir, sample_scans))) + # The default project will have a name which does not match the project in the JSON project = project_factory(import_path=json_file) resp = user_api_client(project=project).post(f'/api/v1/projects/{project.id}/import') if user.is_superuser: assert resp.status_code == 204 + # The import should update the project, even though the names do not match + project.refresh_from_db() + assert project.experiments.count() == 2 + assert project.experiments.all()[0].scans.count() == 1 + assert project.experiments.all()[1].scans.count() == 1 + else: + assert resp.status_code == 401 + + +@pytest.mark.django_db +def test_import_global_json( + tmp_path: Path, + user, + project_factory, + samples_dir: Path, + sample_scans, + user_api_client, +): + json_file = str(tmp_path / 'import.json') + with open(json_file, 'w') as fd: + fd.write(json.dumps(generate_import_json(samples_dir, sample_scans))) + + # noqa:E501 Create a project with the global_import_export flag enabled, but which does not match the project names + project = project_factory( + import_path=json_file, name='projectForImporting', global_import_export=True + ) + # Create projects targeted by the import + project_ohsu = project_factory(import_path=json_file, name='ohsu') + project_ucsd = project_factory(import_path=json_file, name='ucsd') + user_api_client = user_api_client(project=project) + + resp = user_api_client.post(f'/api/v1/projects/{project.id}/import') + if user.is_superuser: + assert resp.status_code == 204 + # The import should update the correctly named projects, but not the original import project + project.refresh_from_db() + project_ohsu.refresh_from_db() + project_ucsd.refresh_from_db() + assert project.experiments.count() == 0 + assert project_ohsu.experiments.count() == 1 + assert project_ohsu.experiments.get().scans.count() == 1 + assert project_ucsd.experiments.count() == 1 + assert project_ucsd.experiments.get().scans.count() == 1 else: assert resp.status_code == 401 From edd3746814756739b25164c0d20050fda6ecd6aa Mon Sep 17 00:00:00 2001 From: Daniel Chiquito Date: Wed, 1 Dec 2021 10:39:28 -0500 Subject: [PATCH 09/10] Add global import/export switch to ProjectSettings --- client/src/components/ProjectSettings.vue | 23 +++++++++++++++++++++++ client/src/types.ts | 3 ++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/client/src/components/ProjectSettings.vue b/client/src/components/ProjectSettings.vue index 643d91aa..aa619f68 100644 --- a/client/src/components/ProjectSettings.vue +++ b/client/src/components/ProjectSettings.vue @@ -17,10 +17,12 @@ export default defineComponent({ const importPath = ref(''); const exportPath = ref(''); + const globalImportExport = ref(false); watchEffect(() => { djangoRest.settings(currentProject.value.id).then((settings) => { importPath.value = settings.importPath; exportPath.value = settings.exportPath; + globalImportExport.value = settings.globalImportExport; }); }); @@ -37,6 +39,7 @@ export default defineComponent({ await djangoRest.setSettings(currentProject.value.id, { importPath: importPath.value, exportPath: exportPath.value, + globalImportExport: globalImportExport.value, }); changed.value = false; } catch (e) { @@ -57,6 +60,7 @@ export default defineComponent({ currentProject, importPath, exportPath, + globalImportExport, changed, importPathError, exportPathError, @@ -120,6 +124,25 @@ export default defineComponent({ /> + + + + Global imports/exports will use the project name from the import file, which will + potentially modify other projects. + + Date: Thu, 2 Dec 2021 09:11:41 -0500 Subject: [PATCH 10/10] Use info icon for global import/export tooltip --- client/src/components/ProjectSettings.vue | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/client/src/components/ProjectSettings.vue b/client/src/components/ProjectSettings.vue index aa619f68..63f3e29d 100644 --- a/client/src/components/ProjectSettings.vue +++ b/client/src/components/ProjectSettings.vue @@ -125,19 +125,23 @@ export default defineComponent({ + Global imports/exports will use the project name from the import file, which will potentially modify other projects.