From 94968e9f8bb50e62dd83cb9f1575d584cef64f42 Mon Sep 17 00:00:00 2001 From: Andy Dirnberger Date: Thu, 30 Jul 2020 09:59:42 -0400 Subject: [PATCH 1/3] Upgrade Pyre to 0.0.56 --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 2cbeb60..9f070f2 100644 --- a/tox.ini +++ b/tox.ini @@ -70,7 +70,7 @@ commands = [testenv:types] deps = factory-boy==3.0.1 - pyre-check==0.0.46 + pyre-check==0.0.56 pytest {[testenv]deps} commands = From 4979da0de874713ce7bf89f50762018c20f080da Mon Sep 17 00:00:00 2001 From: Andy Dirnberger Date: Thu, 30 Jul 2020 10:03:35 -0400 Subject: [PATCH 2/3] Remove unused Pyre ignores After upgrading to Pyre 0.0.48, these `pyre-ignore` comments are no longer needed. --- src/applications/forms.py | 20 ++----------------- .../migrations/0003_auto_20200507_0125.py | 1 - src/applications/models.py | 19 ++++-------------- src/applications/test_forms.py | 1 - src/applications/test_views.py | 5 ----- src/applications/urls.py | 4 ---- src/awards/urls.py | 12 ++--------- src/conftest.py | 2 -- src/homepage/test_views.py | 3 --- src/users/forms.py | 1 - src/users/migrations/0001_initial.py | 5 +---- src/users/models.py | 9 --------- src/users/test_models.py | 1 - src/users/test_views.py | 3 --- src/users/urls.py | 1 - src/users/views.py | 1 - 16 files changed, 9 insertions(+), 79 deletions(-) diff --git a/src/applications/forms.py b/src/applications/forms.py index f067156..6919ca2 100644 --- a/src/applications/forms.py +++ b/src/applications/forms.py @@ -17,9 +17,7 @@ class FinancialAidApplicationForm(ApplicationForm): type = Application.Type.FINANCIAL_AID travel_requested = forms.BooleanField( - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/262. - label=_("Do you need assistance with travel?"), - required=False, + label=_("Do you need assistance with travel?"), required=False, ) class Meta: @@ -32,13 +30,9 @@ class Meta: "lodging_requested", ) labels = { - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/262. "background": _("Tell us a little bit more about yourself"), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/262. "lodging_requested": _("Do you need assistance with lodging?"), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/262. "reason_to_attend": _("Why are you interested in attending PyGotham?"), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/262. "travel_amount": _("What is the estimated cost (USD)?"), } widgets = { @@ -51,12 +45,7 @@ def clean(self) -> Dict[str, Any]: travel_amount = cleaned_data.get("travel_amount") or 0 if travel_amount < 0: raise forms.ValidationError( - { - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/262. - "travel_amount": _( - "Your estimated travel costs cannot be negative." - ) - } + {"travel_amount": _("Your estimated travel costs cannot be negative.")} ) travel_requested = cleaned_data.get("travel_requested") @@ -64,7 +53,6 @@ def clean(self) -> Dict[str, Any]: if not travel_amount: raise forms.ValidationError( { - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/262. "travel_amount": _( "Your estimated travel costs must be greater than $0.00." ) @@ -73,7 +61,6 @@ def clean(self) -> Dict[str, Any]: elif travel_amount: raise forms.ValidationError( { - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/262. "travel_requested": _( "You must request travel assistance before providing an estimated cost." ) @@ -83,7 +70,6 @@ def clean(self) -> Dict[str, Any]: return cleaned_data def clean_lodging_requested(self) -> bool: - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/261. return bool(self.cleaned_data.get("lodging_requested")) @@ -94,9 +80,7 @@ class Meta: model = Application fields = ("background", "reason_to_attend") labels = { - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/262. "background": _("Tell us a little bit about yourself"), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/262. "reason_to_attend": _("Why are you interested in attending PyGotham?"), } diff --git a/src/applications/migrations/0003_auto_20200507_0125.py b/src/applications/migrations/0003_auto_20200507_0125.py index 0284654..6a47770 100644 --- a/src/applications/migrations/0003_auto_20200507_0125.py +++ b/src/applications/migrations/0003_auto_20200507_0125.py @@ -18,7 +18,6 @@ class Migration(migrations.Migration): migrations.AddField( model_name="application", name="travel_amount", - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/260. field=models.DecimalField( blank=True, decimal_places=2, max_digits=10, null=True ), diff --git a/src/applications/models.py b/src/applications/models.py index cec730d..f166a24 100644 --- a/src/applications/models.py +++ b/src/applications/models.py @@ -4,44 +4,33 @@ from django.db import models from django.utils.translation import ugettext_lazy as _ -# pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. User = get_user_model() class Application(models.Model): - # pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. class Status(models.TextChoices): PENDING = "pending" - # pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. class Type(models.TextChoices): FINANCIAL_AID = "finaid" SCHOLARSHIP = "scholarship" - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. + # pyre-ignore[16]: Determine why this ignore is needed. applicant = models.ForeignKey(User, on_delete=models.DO_NOTHING) background = models.TextField(_("applicant background")) reason_to_attend = models.TextField(_("reason the applicant wishes to attend")) status = models.CharField( - max_length=20, - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. - choices=Status.choices, - default=Status.PENDING, + max_length=20, choices=Status.choices, default=Status.PENDING, ) type = models.CharField( - max_length=11, - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. - choices=Type.choices, - default=Type.SCHOLARSHIP, + max_length=11, choices=Type.choices, default=Type.SCHOLARSHIP, ) - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/260. travel_amount = models.DecimalField( max_digits=10, decimal_places=2, blank=True, null=True ) lodging_requested = models.BooleanField(null=True) def __str__(self) -> str: - # pyre-ignore[19]: This is fixed by https://github.com/facebook/pyre-check/pull/256. type_ = Application.Type(self.type) - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. + # pyre-ignore[16]: Determine why this ignore is needed. return f"{type_.label} application for {self.applicant}" diff --git a/src/applications/test_forms.py b/src/applications/test_forms.py index c048ba4..ed07642 100644 --- a/src/applications/test_forms.py +++ b/src/applications/test_forms.py @@ -6,7 +6,6 @@ def test_financial_aid_lodging_requested_is_treated_as_boolean() -> None: form = FinancialAidApplicationForm(other_fields) assert form.is_valid() - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/261. assert form.cleaned_data["lodging_requested"] is False diff --git a/src/applications/test_views.py b/src/applications/test_views.py index 0f138b1..6feff96 100644 --- a/src/applications/test_views.py +++ b/src/applications/test_views.py @@ -24,7 +24,6 @@ class Meta: class UserFactory(DjangoModelFactory): class Meta: - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. model = get_user_model() django_get_or_create = ("email",) @@ -32,7 +31,6 @@ class Meta: @pytest.mark.django_db -# pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. def test_that_one_of_form_type_and_pk_is_required_by_apply(client: Client) -> None: user = UserFactory() qs = get_query_string(user) @@ -46,7 +44,6 @@ def test_that_one_of_form_type_and_pk_is_required_by_apply(client: Client) -> No @pytest.mark.django_db -# pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. def test_user_can_edit_their_application(client: Client) -> None: user = UserFactory() qs = get_query_string(user) @@ -67,7 +64,6 @@ def test_user_can_edit_their_application(client: Client) -> None: @pytest.mark.django_db -# pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. def test_user_can_view_their_application(client: Client) -> None: user = UserFactory() qs = get_query_string(user) @@ -80,7 +76,6 @@ def test_user_can_view_their_application(client: Client) -> None: @pytest.mark.django_db -# pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. def test_user_cant_view_someone_elses_application(client: Client) -> None: user = UserFactory() other = UserFactory(email=f"other+{user.email}") diff --git a/src/applications/urls.py b/src/applications/urls.py index 0dcc925..6bc01c5 100644 --- a/src/applications/urls.py +++ b/src/applications/urls.py @@ -9,19 +9,15 @@ app_name = "applications" urlpatterns = [ - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path("edit/", apply, name="edit"), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path( "financial-aid", apply, {"form_type": FinancialAidApplicationForm}, name="financial_aid", ), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path( "ticket", apply, {"form_type": ScholarshipApplicationForm}, name="scholarship" ), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path("view/", view, name="view"), ] diff --git a/src/awards/urls.py b/src/awards/urls.py index a7ec1cb..f1dce3f 100644 --- a/src/awards/urls.py +++ b/src/awards/urls.py @@ -7,29 +7,22 @@ from users.views import login, magic_login urlpatterns = [ - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path( "", TemplateView.as_view(template_name="homepage/index.html"), name="homepage" ), - # pyre doesn't include stubs for the Django admin. - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path( "admin/login/", RedirectView.as_view( pattern_name=settings.LOGIN_URL, permanent=True, query_string=True ), ), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. + # pyre doesn't include stubs for the Django admin. path("admin/", admin.site.urls), # type: ignore - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path("apply/", include("applications.urls", namespace="applications")), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path("login", login, name="login"), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path("login/magic", magic_login, name="magic-login"), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. + # pyre-ignore[16]: Determine why this ignore is needed. path("logout", LogoutView.as_view(), name="logout"), - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path("users/", include("users.urls", namespace="users")), ] @@ -37,6 +30,5 @@ import debug_toolbar urlpatterns = [ - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path("__debug__/", include(debug_toolbar.urls)), ] + urlpatterns diff --git a/src/conftest.py b/src/conftest.py index 76393b0..0d6bfec 100644 --- a/src/conftest.py +++ b/src/conftest.py @@ -1,7 +1,5 @@ from django.test import Client -# pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. def client() -> Client: - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. return Client() diff --git a/src/homepage/test_views.py b/src/homepage/test_views.py index 645c45a..cba83f2 100644 --- a/src/homepage/test_views.py +++ b/src/homepage/test_views.py @@ -9,7 +9,6 @@ class UserFactory(DjangoModelFactory): class Meta: - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. model = get_user_model() django_get_or_create = ("email",) @@ -17,7 +16,6 @@ class Meta: @pytest.mark.django_db -# pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. def test_login_link_is_not_shown_to_logged_in_users(client: Client) -> None: user = UserFactory() qs = get_query_string(user) @@ -27,7 +25,6 @@ def test_login_link_is_not_shown_to_logged_in_users(client: Client) -> None: assert b"log in" not in response.content.lower() -# pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. def test_login_link_is_shown_to_guests(client: Client) -> None: response = client.get("/") assert b"log in" in response.content.lower() diff --git a/src/users/forms.py b/src/users/forms.py index edc81fe..76411a9 100644 --- a/src/users/forms.py +++ b/src/users/forms.py @@ -4,6 +4,5 @@ class LoginForm(forms.Form): email = forms.EmailField( - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/262. widget=forms.EmailInput(attrs={"placeholder": _("Email address")}) ) diff --git a/src/users/migrations/0001_initial.py b/src/users/migrations/0001_initial.py index d194f7c..3d49e8b 100644 --- a/src/users/migrations/0001_initial.py +++ b/src/users/migrations/0001_initial.py @@ -41,7 +41,6 @@ class Migration(migrations.Migration): ), ( "email", - # pyre-ignore[28]: This is fixed by https://github.com/facebook/pyre-check/pull/256. django.contrib.postgres.fields.citext.CIEmailField( max_length=254, unique=True, verbose_name="email address" ), @@ -49,9 +48,7 @@ class Migration(migrations.Migration): ( "date_joined", models.DateTimeField( - # pyre-ignore[6]: This is fixed by https://github.com/facebook/pyre-check/pull/256. - default=django.utils.timezone.now, - verbose_name="date joined", + default=django.utils.timezone.now, verbose_name="date joined", ), ), ], diff --git a/src/users/models.py b/src/users/models.py index 0ba2d12..c8e4157 100644 --- a/src/users/models.py +++ b/src/users/models.py @@ -7,31 +7,23 @@ from django.utils.translation import ugettext_lazy as _ -# pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/272. class UserManager(BaseUserManager): def create_user(self, email: str, password: str) -> User: - # pyre-ignore[28]: This is fixed by https://github.com/facebook/pyre-check/pull/256. user = User(email=email) - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. user.save() return user def create_superuser(self, email: str, password: str) -> User: - # pyre-ignore[28]: This is fixed by https://github.com/facebook/pyre-check/pull/256. user = User(email=email, is_staff=True) - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. user.save() return user -# pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. class User(AbstractBaseUser): - # pyre-ignore[28]: This is fixed by https://github.com/facebook/pyre-check/pull/256. email = CIEmailField(_("email address"), unique=True) # Blank passwords are allowed since we only allow applicants to use # magic links to log in. password = models.CharField(_("password"), max_length=128, blank=True) - # pyre-ignore[6]: This is fixed by https://github.com/facebook/pyre-check/pull/256. date_joined = models.DateTimeField(_("date joined"), default=timezone.now) is_staff = models.BooleanField(_("user can access the admin"), default=False) @@ -44,7 +36,6 @@ class User(AbstractBaseUser): is_active = True def __str__(self) -> str: - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. username, domain = self.email.split("@", 1) # Hide the characters in the username other than the first one. diff --git a/src/users/test_models.py b/src/users/test_models.py index a60ae52..7b35b96 100644 --- a/src/users/test_models.py +++ b/src/users/test_models.py @@ -6,7 +6,6 @@ pytestmark = pytest.mark.django_db -# pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. User = get_user_model() diff --git a/src/users/test_views.py b/src/users/test_views.py index a7615f3..ff4950f 100644 --- a/src/users/test_views.py +++ b/src/users/test_views.py @@ -6,19 +6,16 @@ TEST_EMAIL = "user@example.org" -# pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. User = get_user_model() @pytest.mark.django_db -# pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. def test_login_creates_new_user(client: Client) -> None: assert not User.objects.filter(email=TEST_EMAIL) client.post("/login", {"email": TEST_EMAIL}) assert User.objects.get(email=TEST_EMAIL) -# pyre-ignore[11]: This is fixed by https://github.com/facebook/pyre-check/pull/256. def test_login_requires_email(client: Client) -> None: response = client.post("/login") assert response.status_code not in (301, 302) diff --git a/src/users/urls.py b/src/users/urls.py index 27449f9..6eeb8a8 100644 --- a/src/users/urls.py +++ b/src/users/urls.py @@ -5,6 +5,5 @@ app_name = "users" urlpatterns = [ - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. path("profile", profile, name="profile"), ] diff --git a/src/users/views.py b/src/users/views.py index a7b0af4..508ddb7 100644 --- a/src/users/views.py +++ b/src/users/views.py @@ -15,7 +15,6 @@ def login(request: HttpRequest) -> HttpResponse: if request.method == "POST": - # pyre-ignore[16]: This is fixed by https://github.com/facebook/pyre-check/pull/256. User = get_user_model() form = LoginForm(request.POST) From d7f3699e57fc17517b99dfab7cc3e75d760d067e Mon Sep 17 00:00:00 2001 From: Andy Dirnberger Date: Thu, 30 Jul 2020 10:07:12 -0400 Subject: [PATCH 3/3] Remove line length ignores from flake8 These line length ignore rules were added to accommodate `pyre-ignore` comments. They are no longer needed and are being removed. Fortunately there was only one line of code that ended up being flagged for being too long. It's being refactored to account for the (once again) enforced limit. --- .flake8 | 15 --------------- src/applications/forms.py | 9 +++------ 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/.flake8 b/.flake8 index 59125dd..ef6809c 100644 --- a/.flake8 +++ b/.flake8 @@ -5,19 +5,4 @@ max-line-length = 80 max-complexity = 18 per-file-ignores = src/awards/settings/dev.py: F405 - # TODO: Remove these once - # https://github.com/facebook/pyre-check/pull/256, - # https://github.com/facebook/pyre-check/pull/260, - # https://github.com/facebook/pyre-check/pull/261, and - # https://github.com/facebook/pyre-check/pull/262 can be used. - src/applications/forms.py: B950 - src/applications/migrations/0003_auto_20200507_0125.py: B950 - src/applications/models.py: B950 - src/applications/test_views.py: B950 - src/awards/urls.py: B950 - src/homepage/test_views.py: B950 - src/users/migrations/0001_initial.py: B950 - src/users/forms.py: B950 - src/users/models.py: B950 - src/users/views.py: B950 select = B,C,E,F,W,T4,B9 diff --git a/src/applications/forms.py b/src/applications/forms.py index 6919ca2..1e7f38a 100644 --- a/src/applications/forms.py +++ b/src/applications/forms.py @@ -59,13 +59,10 @@ def clean(self) -> Dict[str, Any]: } ) elif travel_amount: - raise forms.ValidationError( - { - "travel_requested": _( - "You must request travel assistance before providing an estimated cost." - ) - } + msg = _( + "You must request travel assistance before providing an estimated cost." ) + raise forms.ValidationError({"travel_requested": msg}) return cleaned_data