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

Extensions to django-upgrade #492

Open
UnknownPlatypus opened this issue Sep 22, 2024 · 1 comment
Open

Extensions to django-upgrade #492

UnknownPlatypus opened this issue Sep 22, 2024 · 1 comment

Comments

@UnknownPlatypus
Copy link
Contributor

Description

Hey Adam,

For the past year, I've been using django-upgrade foundations a lot to add some more autofixes rules at work and on some personal projects. They might not all be exactly in scope for django-upgrade (some of them are more like linting rules with autofixes) but I believe they could benefit the Django community. I've been using them in production for almost 6 months now without any issues so far 🤞

The goal of this issue would be to know if you want to me to add some of the fixers to django-upgrade, maybe behind a flag or opt-in using the new rule selection capability. Otherwise, I'm seeking some advice on how to keep my fork up-to-date not to painfully (maybe it should be a package with a dependency on yours?) and package it properly so that some people can use the added fixers -- currently I'm not sure how to do that properly.

Anyway, here is an overview of the fixers I added, if you believe some would be good additions to django-upgrade let me know, I'll be happy to submit PRs (as always). I linked existing PR's in my fork for more details.

1. Always use lazy relationships when defining ForeignKey/ManyToMany fields

A common issue in Django is circular imports when defining foreign keys with direct imports. One workaround Django provides is using a lazy relationship, which this fixer enforces.

from myapp.models import House

class MyModel(models.Model):
-    house = models.ForeignKey(House, null=False, on_delete=models.CASCADE)
+    house = models.ForeignKey("myapp.House", null=False, on_delete=models.CASCADE)

See PR

2. Remove redundant select_related / prefetch_related arguments

In Django, select_related("author__hometown") already includes the author relationship, making select_related("author") redundant. This change removes the duplication and simplifies the query, making the code more concise (Django does the same deduplication work internally at runtime, because it could lead to duplicated SQL queries in some cases).

-Book.objects.select_related("author", "author__hometown")
+Book.objects.select_related("author__hometown")

See PR

3. Reorder Model's inner classes, methods, and fields to follow the Django Style Guide

In large codebase, some Django models tend to become very big.
Discovering the Meta attribute or finding some django specific methods (save, get_absolute_url, ...) can become time consuming when there is no convention enforced.
Some method also usually work together (clean & save for ex) and it make more sense to keep them close.
Manager order also matters and having them all around the place can lead to weird bugs when they are moved around

This mostly implement autofix capability for the existing flake8 linting rule (which is impossible to add to an existing projet if you have to manually fix everything). Careful attention has been given to preserving comments and maintaining the existing style.

The current implementation is not really configurable but uses sane defaults I believe. I've been using it in a team of 10-15 devs for 6 months without any churn yet. It would be really easy to add configurations options though if needed.

See PR

4. Reorder model field kwargs

This fixer standardizes the order of keyword arguments in Django model fields to improve readability and consistency.
By arranging arguments in a consistent order, the code becomes more predictable and making it simpler for developers to quickly identify key attributes in model definitions.

Careful attention has been given to preserving comments and maintaining the existing style.
For now the fixer only format simple django fields. Support for ForeignKey/ManyToMany fiels is not available yet

-data_1 = models.CharField(default="", verbose_name="Donnée 1", blank=True, max_length=191)
-data_2 = models.CharField(blank=True, max_length=191, verbose_name="Donnée 2", default="")
-data_3 = models.CharField(max_length=191, verbose_name="Donnée 3", default="", blank=True)
+data_1 = models.CharField(verbose_name="Donnée 1", default="", blank=True, max_length=191)
+data_2 = models.CharField(verbose_name="Donnée 2", default="", blank=True, max_length=191)
+data_3 = models.CharField(verbose_name="Donnée 3", default="", blank=True, max_length=191)

PR

5. Improve consistency of django.utils.timezone usages

I've seen many people quite confused with when to use timezone.localtime / timezone.make_aware and timezone.localdate leading to a lot of weird pattern that could be achieved in more simple ways. This fixer simplify some of these cases.

-timezone.localtime(aware_date_or_datetime).date()
+timezone.localdate(aware_date_or_datetime)

-timezone.make_aware(dt.datetime.now())
+timezone.localtime()

-timezone.localtime(timezone.now())
+timezone.localtime()
-timezone.localdate(timezone.now())
+timezone.localdate()

-timezone.make_aware(dt.datetime(2022, 4, 21), timezone=timezone.get_current_timezone())
+timezone.make_aware(dt.datetime(2022, 4, 21))
-timezone.localtime(dt.datetime(2022, 4, 21), timezone.get_current_timezone())
+timezone.localtime(dt.datetime(2022, 4, 21))

PR

For curious people

I've also written a few python general purpose fixer that I plan to port to pyupgrade/ruff eventually.

  • Prefer pytest.params(.., id=...) over pytest.mark.parametrize(..., ids=...) ( PR )
  • Use .fromisoformat over .strptime when applicable (~100x faster) ( PR )
  • Convert strftime in f-string to use date format specifier ( PR )

Have a nice day !

@matthiask
Copy link

For the record, 2-5 sound very useful to me as well!

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

No branches or pull requests

2 participants