Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve view-related attribute name consistency #867

Merged
merged 6 commits into from
Jul 13, 2018

Conversation

rpkilby
Copy link
Collaborator

@rpkilby rpkilby commented Jan 27, 2018

Separating out the attribute renaming from #865, since getting the deprecations to work correctly required a bit of work. This required some utility code:

  • Use the RenameMethodsBase metaclass, from django.utils.deprecation
  • Add RenameAttributesBase metaclass (based on RenameMethodsBase)
  • Add MigrationNotice exception, which inherits from DeprecationWarning. This integrates with the above metaclass designs, and allows us to use migration exceptions instead of standard assertions.

Here are the various attribute renames... thoughts?

  • DRF ViewSet.filter_class => filterset_class
  • DRF ViewSet.filter_fields => filterset_fields
  • DjangoFilterBackend.default_filter_set => filterset_base
  • DjangoFilterBackend.get_filter_class() => get_filterset_class()
  • FilterMixin.filter_fields => filterset_fields

Submitting the changes first without the updated tests, to help demonstrate that the metaclasses work.

@codecov-io
Copy link

codecov-io commented Jan 27, 2018

Codecov Report

Merging #867 into master will decrease coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #867      +/-   ##
==========================================
- Coverage   98.25%   98.12%   -0.13%     
==========================================
  Files          15       15              
  Lines        1144     1176      +32     
==========================================
+ Hits         1124     1154      +30     
- Misses         20       22       +2
Impacted Files Coverage Δ
django_filters/utils.py 100% <100%> (+0.97%) ⬆️
django_filters/views.py 100% <100%> (ø) ⬆️
django_filters/rest_framework/backends.py 95.58% <100%> (+0.76%) ⬆️
django_filters/filters.py 98.42% <0%> (-0.97%) ⬇️
django_filters/rest_framework/filterset.py 100% <0%> (ø) ⬆️
django_filters/filterset.py 100% <0%> (ø) ⬆️
django_filters/rest_framework/filters.py 100% <0%> (ø) ⬆️
django_filters/fields.py 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afaf28a...6c1d318. Read the comment docs.

Ryan P Kilby added 6 commits July 13, 2018 10:44
Based on 'django.utils.deprecation.RenameAttributesBase'.
- FilterMixin.filter_fields => filterset_fields
- Backend.default_filter_set => filterset_base
- Backend.get_filter_class => get_filterset_class
- ViewSet.filter_class => filterset_class
- ViewSet.filter_fields => filterset_fields
@carltongibson carltongibson merged commit 04b087d into carltongibson:master Jul 13, 2018
@rpkilby rpkilby deleted the view-renames branch July 13, 2018 13:12
mpw5 added a commit to ministryofjustice/laa-fee-calculator that referenced this pull request Oct 25, 2022
django-filter renamed filter_class to filterset_class some time ago:
carltongibson/django-filter#867. The deprecated
method name remained valid until v22 but now results in failures.

This commit renames all occurences of filter_class to filterset_class.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants