Merge pull request #875 from bodintsov/feature/share-cleanupgrade-2025-type-annotations

[ENG-7443] Feature/share cleanupgrade 2025 type annotations

Author: aaxelb

.github/workflows/run_tests.yml

@@ -9,6 +9,29 @@ permissions:
   checks: write  # for coveralls
 
 jobs:
+  lint_and_type:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install poetry
+        run: pipx install poetry
+
+      - name: setup python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.13'
+          cache: 'poetry'
+
+      - name: install despondencies
+        run: poetry install --with dev
+
+      - name: flake it
+        run: poetry run flake8 .
+
+      - name: type-check
+        run: poetry run mypy trove
+
   run_tests:
     strategy:
       fail-fast: false
@@ -60,9 +83,6 @@ jobs:
       - name: install despondencies
         run: poetry install --with dev
 
-      - name: flake it
-        run: poetry run flake8 .
-
       - name: run tests
         run: |
           poetry run coverage run -m pytest --create-db -x

mypy.ini

@@ -0,0 +1,77 @@
+[mypy]
+python_version = 3.13
+plugins = mypy_django_plugin.main
+
+# display options
+show_column_numbers = True
+pretty = True
+
+# start with an ideal: enable strict type-checking, then loosen in module-specific config
+# see https://mypy.readthedocs.io/en/stable/existing_code.html#introduce-stricter-options
+strict = True
+## BEGIN possible loosenings from `strict`:
+# disallow_subclassing_any = False
+# warn_unused_configs = False
+# warn_redundant_casts = False
+# warn_unused_ignores = False
+# strict_equality = False
+# strict_concatenate = False
+# check_untyped_defs = False
+# disallow_untyped_decorators = False
+# disallow_any_generics = False
+# disallow_untyped_calls = False
+# disallow_incomplete_defs = False
+# disallow_untyped_defs = False
+# no_implicit_reexport = False
+# warn_return_any = False
+## END loosenings of `strict`
+
+# prefer types that can be understood by reading code in only one place
+local_partial_types = True
+# avoid easily-avoidable dead code
+warn_unreachable = True
+# prefer direct imports
+implicit_reexport = False
+
+# got untyped dependencies -- this is fine
+ignore_missing_imports = True
+disable_error_code = import-untyped,import-not-found
+
+###
+# plugin config
+[mypy.plugins.django-stubs]
+django_settings_module = project.settings
+
+###
+# module-specific config
+
+## sharev2 code; largely unannotated
+[mypy-share.*,api.*,project.*,osf_oauth2_adapter.*,manage]
+# loosen strict:
+disallow_subclassing_any = False
+disallow_untyped_decorators = False
+disallow_any_generics = False
+disallow_untyped_calls = False
+disallow_incomplete_defs = False
+disallow_untyped_defs = False
+warn_return_any = False
+
+## django migrations are whatever
+[mypy-*.migrations.*]
+strict = False
+#disable_error_code = var-annotated,import-untyped,import-not-found
+disallow_subclassing_any = False
+
+## tests are looser
+[mypy-tests.*]
+disallow_untyped_defs = False
+
+## trove code (trying to be) well-annotated (but still uses django...)
+[mypy-trove.*]
+disallow_subclassing_any = False
+disallow_untyped_decorators = False
+disallow_any_generics = False
+warn_return_any = False
+[mypy-trove.views.*]
+disallow_untyped_defs = False
+disallow_incomplete_defs = False

poetry.lock

@@ -62,6 +62,8 @@ flake8 = "7.2.0"
 pytest-benchmark = "5.1.0"
 pytest = "8.3.5"
 pytest-django = "4.11.1"
+mypy = "1.16.1"
+django-stubs = "5.2.1"
 
 ###
 # other stuff

pyproject.toml

@@ -39,8 +39,9 @@ def lookups(self, request, model_admin):
         return sorted((x, x.title()) for x in states.ALL_STATES)
 
     def queryset(self, request, queryset):
-        if self.value():
-            return queryset.filter(status=self.value().upper())
+        _value = self.value()
+        if _value:
+            return queryset.filter(status=_value.upper())
         return queryset
 
 
@@ -99,15 +100,15 @@ def status_(self, obj):
             self.STATUS_COLORS.get(obj.status, 'black'),
             obj.status.title()
         )
-    status_.short_description = 'Status'
+    status_.short_description = 'Status'  # type: ignore[attr-defined]
 
     def meta_(self, obj):
         return pprint.pformat(obj.meta)
-    meta_.short_description = 'Meta'
+    meta_.short_description = 'Meta'  # type: ignore[attr-defined]
 
     def source_(self, obj):
         return obj.meta.get('source_config') or obj.meta.get('source')
-    source_.short_description = 'Source'
+    source_.short_description = 'Source'  # type: ignore[attr-defined]
 
     def retry(self, request, queryset):
         for task in queryset:
@@ -116,4 +117,4 @@ def retry(self, request, queryset):
                 task.meta.get('kwargs', {}),
                 task_id=str(task.task_id)
             )
-    retry.short_description = 'Retry Tasks'
+    retry.short_description = 'Retry Tasks'  # type: ignore[attr-defined]

share/admin/celery.py

@@ -1,6 +1,9 @@
-from django.contrib.admin import SimpleListFilter
+from collections.abc import Callable, Sequence
+
+from django.contrib.admin import SimpleListFilter, ModelAdmin
 from django.core.paginator import Paginator
 from django.db import connection, transaction, OperationalError
+from django.db.models import Model
 from django.utils.functional import cached_property
 from django.urls import reverse
 from django.utils.html import format_html
@@ -46,27 +49,31 @@ def admin_link_html(linked_obj):
     return format_html('<a href="{}">{}</a>', url, repr(linked_obj))
 
 
-def linked_fk(field_name):
+def linked_fk[T: type[ModelAdmin]](field_name: str) -> Callable[[T], T]:
     """Decorator that adds a link for a foreign key field
     """
     def add_link(cls):
         def link(self, instance):
             linked_obj = getattr(instance, field_name)
             return admin_link_html(linked_obj)
         link_field = '{}_link'.format(field_name)
-        link.short_description = field_name.replace('_', ' ')
+        link.short_description = field_name.replace('_', ' ')  # type: ignore[attr-defined]
         setattr(cls, link_field, link)
         append_to_cls_property(cls, 'readonly_fields', link_field)
         append_to_cls_property(cls, 'exclude', field_name)
         return cls
     return add_link
 
 
-def linked_many(field_name, order_by=None, select_related=None, defer=None):
-    """Decorator that adds links for a *-to-many field
-    """
-    def add_links(cls):
-        def links(self, instance):
+def linked_many[T: type[ModelAdmin]](
+    field_name: str,
+    order_by: Sequence[str] = (),
+    select_related: Sequence[str] = (),
+    defer: Sequence[str] = (),
+) -> Callable[[T], T]:
+    """Decorator that adds links for a *-to-many field"""
+    def add_links(cls: T) -> T:
+        def links(self, instance: Model) -> str:
             linked_qs = getattr(instance, field_name).all()
             if select_related:
                 linked_qs = linked_qs.select_related(*select_related)
@@ -81,8 +88,8 @@ def links(self, instance):
                     for obj in linked_qs
                 ))
             )
-        links_field = '{}_links'.format(field_name)
-        links.short_description = field_name.replace('_', ' ')
+        links_field = f'{field_name}_links'
+        links.short_description = field_name.replace('_', ' ')  # type: ignore[attr-defined]
         setattr(cls, links_field, links)
         append_to_cls_property(cls, 'readonly_fields', links_field)
         append_to_cls_property(cls, 'exclude', field_name)

share/admin/util.py

@@ -1,3 +1,4 @@
+from __future__ import annotations
 
 from django.db import models
 
@@ -9,13 +10,13 @@
 __all__ = ('SourceConfig',)
 
 
-class SourceConfigManager(models.Manager):
+class SourceConfigManager(models.Manager['SourceConfig']):
     use_in_migrations = True
 
-    def get_by_natural_key(self, key):
+    def get_by_natural_key(self, key) -> SourceConfig:
         return self.get(label=key)
 
-    def get_or_create_push_config(self, user, transformer_key=None):
+    def get_or_create_push_config(self, user, transformer_key=None) -> SourceConfig:
         assert isinstance(user, ShareUser)
         _config_label = '.'.join((
             user.username,

share/models/source_config.py

@@ -19,7 +19,7 @@ class JSONAPIMeta(BaseJSONAPIMeta):
     class Meta:
         unique_together = ('identifier', 'source_config')
 
-    def get_backcompat_sharev2_suid(self):
+    def get_backcompat_sharev2_suid(self) -> 'SourceUniqueIdentifier':
         '''get an equivalent "v2_push" suid for this suid
 
         for filling the legacy suid-based sharev2 index with consistent doc ids

share/models/source_unique_identifier.py

@@ -1,11 +1,14 @@
+import datetime
+from typing import Any
+
 from lxml import etree
 from primitive_metadata import primitive_rdf
 
 from share.util.fromisoformat import fromisoformat
 from trove.vocab.namespaces import OAI, OAI_DC
 
 
-def format_datetime(dt):
+def format_datetime(dt: datetime.datetime | primitive_rdf.Literal | str) -> str:
     """OAI-PMH has specific time format requirements -- comply.
     """
     if isinstance(dt, primitive_rdf.Literal):
@@ -25,15 +28,15 @@ def format_datetime(dt):
 }
 
 
-def ns(namespace_prefix, tag_name):
+def ns(namespace_prefix: str, tag_name: str) -> str:
     """format XML tag/attribute name with full namespace URI
 
     see https://lxml.de/tutorial.html#namespaces
     """
     return f'{{{XML_NAMESPACES[namespace_prefix]}}}{tag_name}'
 
 
-def nsmap(*namespace_prefixes, default=None):
+def nsmap(*namespace_prefixes: str, default: str | None = None) -> dict[str | None, str]:
     """build a namespace map suitable for lxml
 
     see https://lxml.de/tutorial.html#namespaces
@@ -49,7 +52,7 @@ def nsmap(*namespace_prefixes, default=None):
 
 
 # wrapper for lxml.etree.SubElement, adds `text` kwarg for convenience
-def SubEl(parent, tag_name, text=None, **kwargs):
+def SubEl(parent: etree.Element, tag_name: str, text: str | None = None, **kwargs: Any) -> etree.SubElement:
     element = etree.SubElement(parent, tag_name, **kwargs)
     if isinstance(text, primitive_rdf.Literal):
         _language_tag = text.language

share/oaipmh/util.py

@@ -12,7 +12,7 @@
 
 from share.search.messages import MessagesChunk, MessageType
 from share.search import index_strategy
-
+from trove.models import Indexcard
 
 logger = logging.getLogger(__name__)
 
@@ -25,15 +25,15 @@ class IndexMessenger:
         'max_retries': 30,    # give up after 30 tries.
     }
 
-    def __init__(self, *, celery_app=None, index_strategys=None):
+    def __init__(self, *, celery_app=None, index_strategys=None) -> None:
         self.celery_app = (
             celery.current_app
             if celery_app is None
             else celery_app
         )
         self.index_strategys = index_strategys or tuple(index_strategy.each_strategy())
 
-    def notify_indexcard_update(self, indexcards, *, urgent=False):
+    def notify_indexcard_update(self, indexcards: list[Indexcard], *, urgent=False) -> None:
         self.send_messages_chunk(
             MessagesChunk(
                 MessageType.UPDATE_INDEXCARD,
@@ -53,7 +53,7 @@ def notify_indexcard_update(self, indexcards, *, urgent=False):
             urgent=urgent,
         )
 
-    def notify_suid_update(self, suid_ids, *, urgent=False):
+    def notify_suid_update(self, suid_ids, *, urgent=False) -> None:
         self.send_messages_chunk(
             MessagesChunk(MessageType.INDEX_SUID, suid_ids),
             urgent=urgent,