Skip to content

Commit

Permalink
Fix docstrings
Browse files Browse the repository at this point in the history
  • Loading branch information
jraddaoui committed Apr 3, 2019
1 parent 2b3e2dc commit 4d25c0f
Show file tree
Hide file tree
Showing 18 changed files with 100 additions and 109 deletions.
3 changes: 1 addition & 2 deletions dips/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 9 additions & 6 deletions dips/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
29 changes: 13 additions & 16 deletions dips/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand All @@ -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:
Expand All @@ -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()):
Expand All @@ -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))
Expand Down
8 changes: 4 additions & 4 deletions dips/migrations/0002_initial_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
43 changes: 20 additions & 23 deletions dips/models.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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.
"""

Expand Down Expand Up @@ -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.
"""
Expand All @@ -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.
"""
Expand All @@ -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)
Expand All @@ -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

Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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
Expand Down
24 changes: 7 additions & 17 deletions dips/parsemets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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():
Expand All @@ -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(
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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%", "")

Expand All @@ -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).
"""
Expand Down
26 changes: 13 additions & 13 deletions dips/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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_:
Expand All @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion dips/templatetags/custom_tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion dips/tests/test_dip_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down
8 changes: 2 additions & 6 deletions dips/tests/test_get_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand Down
7 changes: 4 additions & 3 deletions dips/tests/test_new_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, {})
Expand Down
Loading

0 comments on commit 4d25c0f

Please sign in to comment.