Skip to content
This repository has been archived by the owner on Oct 2, 2021. It is now read-only.

Upgrade to Pyre 0.0.52 #78

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Upgrade to Pyre 0.0.52 #78

wants to merge 3 commits into from

Conversation

dirn
Copy link
Member

@dirn dirn commented Jul 30, 2020

Unfortunately this upgrade isn't as simple as just updating the version number
in tox.ini. Many of the pyre-ignore comments are now unused (:success:). In
addition to removing them, a couple that I originally thought I could remove now
need to stick around. We have to figure out why and address that.

Upon removing the comments, as per the TODO, the line length for these files no
longer needs to be ignored. The igores can be removed from the flake8
configuration. One line of code is being refactored to account for the line
length now being enforced for its file.

TODO: Address new type errors, either by fixing them or with ignores (which
could potentially change the above).

@dirn dirn changed the title Upgrade to Pyre 0.0.48 Upgrade to Pyre 0.0.52 Aug 3, 2020
@dirn
Copy link
Member Author

dirn commented Aug 3, 2020

Hey @gbleaney sorry to pull you into this, but do you have a minute to take a look at the failing types job? Most of the failures seem to fall into two categories:

  1. types handled by __getattr__ and
  2. redirected imports (e.g., from django.urls.base import reverse works but from django.urls import reverse does not).

These all worked under pyre-check 0.0.46 but started failing in 0.0.48. I can try 0.0.47 if you think that will help but haven't yet since it isn't on PyPI.

(The pytest-related failures could be related to pytest 6.0.0 adding annotations. I haven't dug into those yet.)

After upgrading to Pyre 0.0.48, these `pyre-ignore` comments are no
longer needed.
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.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant