Skip to content

Commit

Permalink
Merge pull request #339 from intgr-forks/initial-type-hints
Browse files Browse the repository at this point in the history
Add initial type hints and run mypy in CI
  • Loading branch information
alanjds authored May 10, 2024
2 parents 4b1350d + 1595a77 commit ffce21b
Show file tree
Hide file tree
Showing 13 changed files with 160 additions and 67 deletions.
25 changes: 24 additions & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,27 @@ jobs:
echo
env
- name: Test with tox
run: tox
run: tox run --skip-env=py311-mypy

mypy:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
with:
python-version: 3.11
- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install tox tox-gh-actions
- name: Know your environment
run: |
cd $GITHUB_WORKSPACE
echo
ls -F
echo
env
- name: Test with tox
run: tox run -e py311-mypy
14 changes: 14 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[mypy]
check_untyped_defs = true
disallow_incomplete_defs = true
disallow_untyped_defs = true
fast_module_lookup = true
ignore_missing_imports = true
implicit_reexport = false
local_partial_types = true
show_column_numbers = true
show_error_codes = true
strict_equality = true
warn_redundant_casts = true
warn_unreachable = true
warn_unused_ignores = true
5 changes: 5 additions & 0 deletions requirements-tox.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ flake8==6.1.0
ipdb==0.13.13
pytz==2024.1

# Type checking requirements
mypy==1.10.0
django-stubs==5.0.0
djangorestframework-stubs==3.15.0

# wheel for PyPI installs
wheel==0.43.0

Expand Down
32 changes: 20 additions & 12 deletions rest_framework_nested/relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,32 @@
These fields allow you to specify the style that should be used to represent
model relationships with hyperlinks.
"""
from __future__ import annotations

from functools import reduce
from typing import Any, Generic, TypeVar

import rest_framework.relations
from rest_framework.relations import ObjectDoesNotExist, ObjectValueError, ObjectTypeError
from django.core.exceptions import ObjectDoesNotExist
from django.db.models import Model
from rest_framework.relations import HyperlinkedRelatedField, ObjectTypeError, ObjectValueError
from rest_framework.exceptions import ValidationError
from rest_framework.request import Request


T_Model = TypeVar('T_Model', bound=Model)


class NestedHyperlinkedRelatedField(rest_framework.relations.HyperlinkedRelatedField):
class NestedHyperlinkedRelatedField(HyperlinkedRelatedField, Generic[T_Model]):
lookup_field = 'pk'
parent_lookup_kwargs = {
'parent_pk': 'parent__pk'
}

def __init__(self, *args, **kwargs):
def __init__(self, *args: Any, **kwargs: Any) -> None:
self.parent_lookup_kwargs = kwargs.pop('parent_lookup_kwargs', self.parent_lookup_kwargs)
super().__init__(*args, **kwargs)

def get_url(self, obj, view_name, request, format):
def get_url(self, obj: Model, view_name: str, request: Request, format: str | None) -> str | None:
"""
Given an object, return the URL that hyperlinks to the object.
Expand All @@ -46,7 +54,7 @@ def get_url(self, obj, view_name, request, format):

try:
# use the Django ORM to lookup this value, e.g., obj.parent.pk
lookup_value = reduce(getattr, [obj] + lookups)
lookup_value = reduce(getattr, [obj] + lookups) # type: ignore[operator,arg-type]
except AttributeError:
# Not nested. Act like a standard HyperlinkedRelatedField
return super().get_url(obj, view_name, request, format)
Expand All @@ -56,7 +64,7 @@ def get_url(self, obj, view_name, request, format):

return self.reverse(view_name, kwargs=kwargs, request=request, format=format)

def get_object(self, view_name, view_args, view_kwargs):
def get_object(self, view_name: str, view_args: list[Any], view_kwargs: dict[str, Any]) -> T_Model:
"""
Return the object corresponding to a matched URL.
Expand All @@ -74,14 +82,14 @@ def get_object(self, view_name, view_args, view_kwargs):

return self.get_queryset().get(**kwargs)

def use_pk_only_optimization(self):
def use_pk_only_optimization(self) -> bool:
return False

def to_internal_value(self, data):
def to_internal_value(self, data: Any) -> T_Model:
try:
return super().to_internal_value(data)
except ValidationError as err:
if err.detail[0].code != 'no_match':
if err.detail[0].code != 'no_match': # type: ignore[union-attr,index]
raise

# data is probable the lookup value, not the resource URL
Expand All @@ -91,8 +99,8 @@ def to_internal_value(self, data):
self.fail('does_not_exist')


class NestedHyperlinkedIdentityField(NestedHyperlinkedRelatedField):
def __init__(self, view_name=None, **kwargs):
class NestedHyperlinkedIdentityField(NestedHyperlinkedRelatedField[T_Model]):
def __init__(self, view_name: str | None = None, **kwargs: Any) -> None:
assert view_name is not None, 'The `view_name` argument is required.'
kwargs['read_only'] = True
kwargs['source'] = '*'
Expand Down
37 changes: 24 additions & 13 deletions rest_framework_nested/routers.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@
urlpatterns = router.urls
"""

from __future__ import annotations

import sys
import re
from rest_framework.routers import SimpleRouter, DefaultRouter # noqa: F401
from typing import Any

from rest_framework.routers import DefaultRouter, SimpleRouter

if sys.version_info[0] < 3:
IDENTIFIER_REGEX = re.compile(r"^[^\d\W]\w*$")
Expand All @@ -45,7 +48,13 @@ class LookupMixin:


class NestedMixin:
def __init__(self, parent_router, parent_prefix, *args, **kwargs):
def __init__(
self,
parent_router: SimpleRouter | DefaultRouter | NestedMixin,
parent_prefix: str,
*args: Any,
**kwargs: Any
) -> None:
self.parent_router = parent_router
self.parent_prefix = parent_prefix
self.nest_count = getattr(parent_router, 'nest_count', 0) + 1
Expand All @@ -67,21 +76,23 @@ def __init__(self, parent_router, parent_prefix, *args, **kwargs):
# behavior is ALWAYS consistent with the parent. If we didn't, we might create
# a situation where the parent's trailing slash is truthy (but not '/') and
# we set our trailing slash to just '/', leading to inconsistent behavior.
self.trailing_slash = parent_router.trailing_slash
self.trailing_slash = parent_router.trailing_slash # type: ignore[has-type]

parent_registry = [registered for registered
in self.parent_router.registry
if registered[0] == self.parent_prefix]
parent_registry = [
registered for registered
in self.parent_router.registry # type: ignore[union-attr]
if registered[0] == self.parent_prefix
]
try:
parent_registry = parent_registry[0]
parent_prefix, parent_viewset, parent_basename = parent_registry
parent_registry_item = parent_registry[0]
parent_prefix, parent_viewset, parent_basename = parent_registry_item
except:
raise RuntimeError('parent registered resource not found')

self.check_valid_name(self.nest_prefix)

nested_routes = []
parent_lookup_regex = parent_router.get_lookup_regex(parent_viewset, self.nest_prefix)
parent_lookup_regex = parent_router.get_lookup_regex(parent_viewset, self.nest_prefix) # type: ignore[union-attr]

self.parent_regex = f'{parent_prefix}/{parent_lookup_regex}/'
# If there is no parent prefix, the first part of the url is probably
Expand All @@ -93,7 +104,7 @@ def __init__(self, parent_router, parent_prefix, *args, **kwargs):
if hasattr(parent_router, 'parent_regex'):
self.parent_regex = parent_router.parent_regex + self.parent_regex

for route in self.routes:
for route in self.routes: # type: ignore[has-type]
route_contents = route._asdict()

# This will get passed through .format in a little bit, so we need
Expand All @@ -105,12 +116,12 @@ def __init__(self, parent_router, parent_prefix, *args, **kwargs):

self.routes = nested_routes

def check_valid_name(self, value):
def check_valid_name(self, value: str) -> None:
if IDENTIFIER_REGEX.match(value) is None:
raise ValueError(f"lookup argument '{value}' needs to be valid python identifier")


class NestedSimpleRouter(NestedMixin, SimpleRouter):
class NestedSimpleRouter(NestedMixin, SimpleRouter): # type: ignore[misc]
""" Create a NestedSimpleRouter nested within `parent_router`
Args:
Expand All @@ -131,7 +142,7 @@ class NestedSimpleRouter(NestedMixin, SimpleRouter):
pass


class NestedDefaultRouter(NestedMixin, DefaultRouter):
class NestedDefaultRouter(NestedMixin, DefaultRouter): # type: ignore[misc]
""" Create a NestedDefaultRouter nested within `parent_router`
Args:
Expand Down
6 changes: 4 additions & 2 deletions rest_framework_nested/runtests/runcoverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@
# http://code.djangoproject.com/svn/django/trunk/tests/runtests.py
import os
import sys
from typing import NoReturn
from coverage import coverage

# fix sys path so we don't need to setup PYTHONPATH
sys.path.append(os.path.join(os.path.dirname(__file__), "../.."))
os.environ['DJANGO_SETTINGS_MODULE'] = 'rest_framework_nested.runtests.settings'


def main():
def main() -> NoReturn:
"""Run the tests for rest_framework and generate a coverage report."""

cov = coverage()
Expand All @@ -26,6 +27,7 @@ def main():
from django.test.utils import get_runner
TestRunner = get_runner(settings)

failures: int
if hasattr(TestRunner, 'func_name'):
# Pre 1.2 test runners were just functions,
# and did not support the 'failfast' option.
Expand All @@ -34,7 +36,7 @@ def main():
'Function-based test runners are deprecated. Test runners should be classes with a run_tests() method.',
DeprecationWarning
)
failures = TestRunner(['tests'])
failures = TestRunner(['tests']) # type: ignore[assignment,arg-type]
else:
test_runner = TestRunner()
failures = test_runner.run_tests(['tests'])
Expand Down
5 changes: 3 additions & 2 deletions rest_framework_nested/runtests/runtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# http://code.djangoproject.com/svn/django/trunk/tests/runtests.py
import os
import sys
from typing import NoReturn

# fix sys path so we don't need to setup PYTHONPATH
sys.path.append(os.path.join(os.path.dirname(__file__), "../.."))
Expand All @@ -15,7 +16,7 @@
from django.test.utils import get_runner


def usage():
def usage() -> str:
return """
Usage: python runtests.py [UnitTestClass].[method]
Expand All @@ -25,7 +26,7 @@ def usage():
"""


def main():
def main() -> NoReturn:
TestRunner = get_runner(settings)
import ipdb
ipdb.set_trace()
Expand Down
18 changes: 10 additions & 8 deletions rest_framework_nested/runtests/settings.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

# Django settings for testproject project.

DEBUG = True
Expand Down Expand Up @@ -83,7 +85,7 @@
# Don't forget to use absolute paths, not relative paths.
)

INSTALLED_APPS = (
INSTALLED_APPS = [
'django.contrib.auth',
'django.contrib.contenttypes',
'django.contrib.sessions',
Expand All @@ -96,7 +98,7 @@
# 'rest_framework.authtoken',
'rest_framework_nested',
'rest_framework_nested.tests',
)
]

# OAuth is optional and won't work if there is no oauth_provider & oauth2
try:
Expand All @@ -105,19 +107,19 @@
except ImportError:
pass
else:
INSTALLED_APPS += (
INSTALLED_APPS += [
'oauth_provider',
)
]

try:
import provider # noqa: F401
except ImportError:
pass
else:
INSTALLED_APPS += (
INSTALLED_APPS += [
'provider',
'provider.oauth2',
)
]

# guardian is optional
try:
Expand All @@ -130,9 +132,9 @@
'django.contrib.auth.backends.ModelBackend', # default
'guardian.backends.ObjectPermissionBackend',
)
INSTALLED_APPS += (
INSTALLED_APPS += [
'guardian',
)
]

STATIC_URL = '/static/'

Expand Down
2 changes: 1 addition & 1 deletion rest_framework_nested/runtests/urls.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""
Blank URLConf just to keep runtests.py happy.
"""
from rest_framework.compat import patterns
from rest_framework.compat import patterns # type: ignore[attr-defined]

urlpatterns = patterns('',)
Loading

0 comments on commit ffce21b

Please sign in to comment.