From 4d25c0fad9060a54bfeac435ffb1533f1fb27c29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Raddaoui=20Mar=C3=ADn?= Date: Wed, 3 Apr 2019 18:57:35 +0200 Subject: [PATCH] Fix docstrings --- dips/forms.py | 3 +- dips/handlers.py | 15 ++++++---- dips/helpers.py | 29 +++++++++---------- dips/migrations/0002_initial_data.py | 8 +++--- dips/models.py | 43 +++++++++++++--------------- dips/parsemets.py | 24 +++++----------- dips/tasks.py | 26 ++++++++--------- dips/templatetags/custom_tags.py | 3 +- dips/tests/test_dip_download.py | 3 +- dips/tests/test_get_users.py | 8 ++---- dips/tests/test_new_collection.py | 7 +++-- dips/tests/test_user_access.py | 18 ++++++++---- dips/views.py | 3 +- scope/__init__.py | 7 +++-- scope/settings.py | 3 +- scope/staticfiles.py | 3 +- scope/urls.py | 3 +- scope/wsgi.py | 3 +- 18 files changed, 100 insertions(+), 109 deletions(-) diff --git a/dips/forms.py b/dips/forms.py index 82bbad5a..a9725a73 100644 --- a/dips/forms.py +++ b/dips/forms.py @@ -125,8 +125,7 @@ def save(self, commit=True): class SettingsForm(forms.Form): - """ - Base form for `models.Setting` management: + """Base form for `models.Setting` management. - Field names declared in sub-forms must match the setting names. - The field type and other properties must be defined in the sub-forms diff --git a/dips/handlers.py b/dips/handlers.py index bab2cc26..52bb25c2 100644 --- a/dips/handlers.py +++ b/dips/handlers.py @@ -7,11 +7,14 @@ @receiver(pre_delete, sender=Collection, dispatch_uid="collection_pre_delete") @receiver(pre_delete, sender=DIP, dispatch_uid="dip_pre_delete") def delete_related_dc(instance, **kwargs): - # When cascade deleting related models the custom delete method from - # the model classes is not called. The DublinCore models are not deleted - # in the cascade process because they're related to both Collection and DIP - # models from their respective tables due to the lack of a base model, - # where the relation could be made from the DublinCore model to the base - # model and use on delete cascade for that one to one relation. + """Delete related DublinCore handler. + + When cascade deleting related models the custom delete method from + the model classes is not called. The DublinCore models are not deleted + in the cascade process because they're related to both Collection and DIP + models from their respective tables due to the lack of a base model, + where the relation could be made from the DublinCore model to the base + model and use on delete cascade for that one to one relation. + """ if instance.dc: instance.dc.delete() diff --git a/dips/helpers.py b/dips/helpers.py index 1a96a84b..bfe92b4a 100644 --- a/dips/helpers.py +++ b/dips/helpers.py @@ -11,9 +11,9 @@ def add_if_not_empty(data_dict, key, value): def convert_size(size): - """ - Convert size to human-readable form using base 2. Should this be using - using base 10? https://wiki.ubuntu.com/UnitsPolicy + """Convert size to human-readable form using base 2. + + Should this be using using base 10? https://wiki.ubuntu.com/UnitsPolicy """ size_name = ("bytes", "KB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB") i = int(math.floor(math.log(size, 1024))) @@ -25,10 +25,7 @@ def convert_size(size): def update_instance_from_dict(instance, dict): - """ - Update model instance attributes from dict key, value pairs. - Return the updated instance without saving. - """ + """Update instance attributes from dict key, value pairs without saving.""" for field, value in dict.items(): # Check field existence in model instance try: @@ -40,10 +37,10 @@ def update_instance_from_dict(instance, dict): def get_sort_params(params, options, default): - """ - Get sort option and direction from params. Check options dict. for - available choices and use default if no option is passed on the params - or if the option is not valid. Defaults to asc. sort direction. + """Get sort option and direction from params. + + Check options dict. for available choices and use default if no option is passed + on the paramsor if the option is not valid. Defaults to asc. sort direction. """ option = params.get("sort", default) if option not in list(options.keys()): @@ -57,11 +54,11 @@ def get_sort_params(params, options, default): def get_page_from_search(search, params): - """ - Create paginator and return current page based on the current search - and the page and limit parameters. Limit defaults to 10 and - can't be set over 100. The search parameter can be an Elasticsearch - search or a Django model QuerySet. + """Create paginator and return current page. + + Based on the current search and the page and limit parameters. Limit + defaults to 10 and can't be set over 100. The search parameter can be an + Elasticsearch search or a Django model QuerySet. """ try: limit = int(params.get("limit", 10)) diff --git a/dips/migrations/0002_initial_data.py b/dips/migrations/0002_initial_data.py index 878c4868..24822a28 100644 --- a/dips/migrations/0002_initial_data.py +++ b/dips/migrations/0002_initial_data.py @@ -9,10 +9,10 @@ def migrate_permissions(apps, schema_editor): - """ - Create permissions in migration before they are created in a - post_migrate signal handler. They are needed to create the user - group and the signal handler won't create duplicated permissions. + """Create permissions in migrations. + + Needed before they are created in a post_migrate signal handler to create + the user groups. The signal handler won't create duplicated permissions. This must be executed after the auth and dips apps are migrated. """ for app_config in apps.get_app_configs(): diff --git a/dips/models.py b/dips/models.py index d95a5d5b..31234160 100644 --- a/dips/models.py +++ b/dips/models.py @@ -1,5 +1,4 @@ -""" -Model classes declaration for dips app: +"""Model classes declaration for dips app: To connect Django models to elasticsearch-dsl documents declared in search.documents, an AbstractEsModel has been created with the ABC and @@ -46,13 +45,13 @@ def is_manager(self): @classmethod def get_users(cls, query=None, sort_field="username"): - """ - Get users based on a query string, querying over 'username', - 'first_name', 'last_name', 'email' and a concatenation of related - group names separated by ', '. The group name concatenation - can be used to sort and display in the 'group_names' field and the - output will be the same as the equally called function from this model. - The resulting users will be ordered by a given 'sort_field'. Returns + """Get users based on a query string. + + Querying over 'username', 'first_name', 'last_name', 'email' and a + concatenation of related group names separated by ', '. The group name + concatenation can be used to sort and display in the 'group_names' field + and the output will be the same as the equally called function from this + model. The resulting users will be ordered by a given 'sort_field'. Returns all users if no query is given and sorts by 'username' by default. """ @@ -185,7 +184,8 @@ def __str__(self): return self.identifier def get_es_inner_data(self): - """ + """Get data for inner objects in other ES docs. + Returns a dictionary with field name > value with the required data to be stored in the Elasticsearch documents of related models. """ @@ -196,7 +196,8 @@ def get_es_inner_data(self): return data def get_display_data(self): - """ + """Get ordered data for display. + Returns a dictionary with display label > value from object fields, checking the enabled fields and the hide empty fields configuration. """ @@ -215,9 +216,7 @@ def get_display_data(self): @classmethod def get_optional_fields(cls): - """ - Returns a dictionary with optional fields name > verbose_name. - """ + """Returns a dictionary with optional fields name > verbose_name.""" optional_fields = OrderedDict() for field_name in cls.ORDERED_FIELDS: field = cls._meta.get_field(field_name) @@ -228,18 +227,16 @@ def get_optional_fields(cls): @classmethod def enabled_fields(cls): - """ - Returns a list with enabled field names based on - `enabled_dc_fields` setting. + """Returns a list with enabled field names. + + Based on `enabled_dc_fields` setting. """ setting = Setting.objects.get(name="enabled_optional_dc_fields") return cls.REQUIRED_FIELDS + setting.value @classmethod def hide_empty_fields(cls): - """ - Returns a boolean based on `hide_empty_dc_fields` setting. - """ + """Returns a boolean based on `hide_empty_dc_fields` setting.""" setting = Setting.objects.get(name="hide_empty_dc_fields") return setting.value @@ -362,7 +359,8 @@ def get_import_error_message(self): ) def is_visible_by_user(self, user): - """ + """Check if a user can see the instance. + Retrun `False` if there is an import pending for the DIP or if the import failed and the user is not an editor or an admin. Otherwise, return `True`. @@ -438,8 +436,7 @@ def __str__(self): class Setting(models.Model): - """ - Name/value pairs for application settings. + """Name/value pairs for application settings. A database-agnostic JSONField is used for the `value` field with auto encoding/decoding but without extended querying capabilities. If new diff --git a/dips/parsemets.py b/dips/parsemets.py index 88c20c34..be9582a1 100644 --- a/dips/parsemets.py +++ b/dips/parsemets.py @@ -16,9 +16,7 @@ class METSError(Exception): class METS(object): - """ - Class for METS file parsing methods. - """ + """Class for METS file parsing methods.""" # Fields and xpaths for DigitalFile FILE_ELEMENTS = [ @@ -75,9 +73,7 @@ def __str__(self): return self.path def _get_mets_root(self): - """ - Open XML and return the root element with all namespaces stripped. - """ + """Open XML and return the root element with all namespaces stripped.""" tree = etree.parse(self.path) root = tree.getroot() for elem in root.getiterator(): @@ -90,9 +86,7 @@ def _get_mets_root(self): return root def parse_mets(self): - """ - Parse METS and save data to DIP, DigitalFile, and PremisEvent models. - """ + """Parse METS and save data to DIP, DigitalFile, and PremisEvent models.""" # Get DIP object dip = DIP.objects.get(pk=self.dip_id) logger.info( @@ -192,9 +186,7 @@ def parse_mets(self): logger.info("No DIP Dublin Core metadata found") def _parse_file_metadata(self, amdsec_id): - """ - Parse file metadata into a dict and an events list. - """ + """Parse file metadata into a dict and an events list.""" # Create new dictionary for this item's info, including # the amdSec id, and new list of dicts for premis events. data = {"amdsec": amdsec_id} @@ -227,9 +219,7 @@ def _parse_file_metadata(self, amdsec_id): return (data, events) def _transform_file_metadata(self, data): - """ - Transform file metadata to be saved in DigitalFile fields. - """ + """Transform file metadata to be saved in DigitalFile fields.""" # Format filepath data["filepath"] = data["filepath"].replace("%transferDirectory%", "") @@ -251,8 +241,8 @@ def _transform_file_metadata(self, data): return data def _parse_dc(self): - """ - Parse SIP-level Dublin Core metadata into dc_model dictionary. + """Parse SIP-level Dublin Core metadata into dc_model dictionary. + Based on `parse_dc` from Archivematica parse_mets_to_db.py script (src/MCPClient/lib/clientScripts/parse_mets_to_db.py). """ diff --git a/dips/tasks.py b/dips/tasks.py index 3f8ad90a..0856bdbe 100644 --- a/dips/tasks.py +++ b/dips/tasks.py @@ -20,10 +20,10 @@ class MetsTask(Task): def after_return(self, status, retval, task_id, args, kwargs, einfo): - """ - Update DIP `import_status` when the task ends. Make sure it's one - of Celery's READY_STATES and set it to 'FAILURE' for all possible - non 'SUCCESS' states. + """Update DIP `import_status` when the task ends. + + Make sure it's one of Celery's READY_STATES and set it to 'FAILURE' for + all possible non 'SUCCESS' states. """ if status not in states.READY_STATES: return @@ -45,12 +45,12 @@ def after_return(self, status, retval, task_id, args, kwargs, einfo): default_retry_delay=30, ) def extract_and_parse_mets(dip_id, zip_path): - """ - Extracts a METS file from a given DIP zip file and uses the METS class - to parse its content, create the related DigitalFiles and update the DIP - DC metadata. Creates and deletes a temporary directory to hold the METS - file during its parsing. This function is meant to be called with - `.delay()` to be executed asynchronously by the Celery worker. + """Extract and parse a METS file from a given DIP zip file. + + Uses the METS class to parse its content, create the related DigitalFiles + and update the DIP DC metadata. Creates and deletes a temporary directory + to hold the METS file during its parsing. This function is meant to be called + with `.delay()` to be executed asynchronously by the Celery worker. """ logger.info("Extracting METS file from ZIP [Path: %s]" % zip_path) with tempfile.TemporaryDirectory() as dir_: @@ -77,9 +77,9 @@ def extract_and_parse_mets(dip_id, zip_path): ignore_result=True, ) def update_es_descendants(class_name, pk): - """ - Updates the related DigitalFiles documents in ES with the partial data from - the ancestor Collection or DIP. + """Update the related DigitalFiles documents in ES. + + With the partial data from the ancestor Collection or DIP. """ if class_name not in ["Collection", "DIP"]: raise Exception("Can not update descendants of %s." % class_name) diff --git a/dips/templatetags/custom_tags.py b/dips/templatetags/custom_tags.py index 3dbd842b..7cdb3dec 100644 --- a/dips/templatetags/custom_tags.py +++ b/dips/templatetags/custom_tags.py @@ -12,7 +12,8 @@ def sort_by(queryset, order): @register.simple_tag def update_and_encode_params(querydict, *args, **kwargs): - """ + """Format parameters for relative URL. + Copy the request GET QueryDict and update it with the key/value pairs sent in kwargs. Removes key if the value sent is None. Return the updated parameters encoded to use in a relative URL. diff --git a/dips/tests/test_dip_download.py b/dips/tests/test_dip_download.py index 24afcf24..1dade2b2 100644 --- a/dips/tests/test_dip_download.py +++ b/dips/tests/test_dip_download.py @@ -11,7 +11,8 @@ @contextmanager def _sized_tmp_file(path, size): - """ + """Manage sized temporary file. + Context manager that creates a file (raising FileExistsError if it exists) in a given path and with a given size, yields it and deletes it at the end. """ diff --git a/dips/tests/test_get_users.py b/dips/tests/test_get_users.py index c30bd966..c39c30f0 100644 --- a/dips/tests/test_get_users.py +++ b/dips/tests/test_get_users.py @@ -47,9 +47,7 @@ def setUpTestData(cls): user.groups.add(group) def test_get_users_no_params(self): - """ - Get all users without filtering, sorted by default (username). - """ + """Get all users without filtering, sorted by default (username).""" users = User.get_users() self.assertEqual(users.count(), 4) first_user = users[0] @@ -58,9 +56,7 @@ def test_get_users_no_params(self): self.assertEqual(last_user.username, "MariaPerez") def test_get_users_sort_by_group(self): - """ - Get all users, sorted by group names concatenation. - """ + """Get all users, sorted by group names concatenation.""" users = User.get_users(sort_field="group_names") self.assertEqual(users.count(), 4) first_user = users[0] diff --git a/dips/tests/test_new_collection.py b/dips/tests/test_new_collection.py index 4fa60190..f6107931 100644 --- a/dips/tests/test_new_collection.py +++ b/dips/tests/test_new_collection.py @@ -35,9 +35,10 @@ def test_new_topic_valid_post_data(self, mock_es_save): self.assertTrue(collection) def test_new_topic_invalid_post_data_empty_fields(self): - """ - Invalid post data should not redirect - The expected behavior is to show the form again with validation errors + """Test invalid post data. + + Invalid post data should not redirect. The expected behavior + is to show the form again with validation errors. """ url = reverse("new_collection") response = self.client.post(url, {}) diff --git a/dips/tests/test_user_access.py b/dips/tests/test_user_access.py index 4994d331..df18225e 100644 --- a/dips/tests/test_user_access.py +++ b/dips/tests/test_user_access.py @@ -188,7 +188,8 @@ def setUp(self, mock_es_save): @patch("elasticsearch_dsl.Search.execute") @patch("elasticsearch_dsl.Search.count", return_value=0) def test_get_pages(self, mock_es_count, mock_es_exec): - """ + """Get pages test. + Makes get requests to pages with different user types logged in and verifies if the user can see the page or gets redirected. """ @@ -212,7 +213,8 @@ def test_get_pages(self, mock_es_count, mock_es_exec): self.client.logout() def test_post_user(self): - """ + """Post user test. + Makes post requests to create and edit user pages with different user types logged in and verifies the results. """ @@ -310,7 +312,8 @@ def test_post_user(self): @patch("elasticsearch_dsl.DocType.save") @patch("dips.models.celery_app.send_task") def test_post_collection(self, mock_send_task, mock_es_save): - """ + """Post collection test. + Makes post requests to create and edit collection pages with different user types logged in and verifies the results. """ @@ -410,7 +413,8 @@ def test_post_collection(self, mock_send_task, mock_es_save): @patch("elasticsearch_dsl.DocType.save") @patch("dips.models.celery_app.send_task") def test_post_dip(self, mock_send_task, mock_es_save): - """ + """Post DIP test. + Makes post requests to create and edit DIP pages with different user types logged in and verifies the results. """ @@ -499,7 +503,8 @@ def test_post_dip(self, mock_send_task, mock_es_save): @patch("dips.models.delete_document") @patch("dips.models.celery_app.send_task") def test_delete_dip(self, mock_send_task, mock_es_delete): - """ + """Delete DIP test. + Makes post request to delete a DIP with different user types logged in and verifies the results. """ @@ -558,7 +563,8 @@ def test_delete_dip(self, mock_send_task, mock_es_delete): @patch("dips.models.delete_document") @patch("dips.models.celery_app.send_task") def test_delete_collection(self, mock_send_task, mock_es_delete): - """ + """Delete collection test. + Makes post request to delete a collection with different user types logged in and verifies the results. """ diff --git a/dips/views.py b/dips/views.py index 987cf641..0252ed2e 100644 --- a/dips/views.py +++ b/dips/views.py @@ -23,7 +23,8 @@ def _get_and_validate_digital_file_filters(request): - """ + """Process digital file filters. + Obtains the digital file filters from the request GET parameteres and validates the start and end dates. Returns two dics, the first one with all the filters set (to maintain their value in the templates) and the diff --git a/scope/__init__.py b/scope/__init__.py index 8d1c7bb7..c9122262 100644 --- a/scope/__init__.py +++ b/scope/__init__.py @@ -1,6 +1,7 @@ -""" -This will make sure the Celery app is always imported -when Django starts so that shared_task will use it. +"""SCOPE application. + +Make sure the Celery app is always imported when Django starts +so that shared_task will use it. """ from .celery import app as celery_app diff --git a/scope/settings.py b/scope/settings.py index c023834a..03d6ffe5 100644 --- a/scope/settings.py +++ b/scope/settings.py @@ -1,5 +1,4 @@ -""" -Django settings for scope project. +"""Django settings for scope project. Generated by 'django-admin startproject' using Django 1.11.5. diff --git a/scope/staticfiles.py b/scope/staticfiles.py index a688c763..8065fcca 100644 --- a/scope/staticfiles.py +++ b/scope/staticfiles.py @@ -2,7 +2,8 @@ class Config(StaticFilesConfig): - """ + """Static files configuration. + The static files matching the following patterns are needed in the static folders to access them in development mode and by `django-compress` but they are not needed by `collectstatic`, where the compressed bundles should diff --git a/scope/urls.py b/scope/urls.py index bcbdca07..2188ac1a 100644 --- a/scope/urls.py +++ b/scope/urls.py @@ -1,5 +1,4 @@ -""" -scope URL Configuration +"""SCOPE URL Configuration The `urlpatterns` list routes URLs to views. For more information please see: https://docs.djangoproject.com/en/1.11/topics/http/urls/ diff --git a/scope/wsgi.py b/scope/wsgi.py index f278d64f..74963476 100644 --- a/scope/wsgi.py +++ b/scope/wsgi.py @@ -1,5 +1,4 @@ -""" -WSGI config for scope project. +"""WSGI config for SCOPE project. It exposes the WSGI callable as a module-level variable named ``application``.