Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

isort 5 compatibility #7786

Merged
merged 6 commits into from
Jul 5, 2020
Merged

isort 5 compatibility #7786

merged 6 commits into from
Jul 5, 2020

Conversation

Half-Shot
Copy link
Collaborator

The CI appears to use the latest version of isort, which is a problem when isort gets a major version bump. Rather than try to pin the version, I've done the necessary to make isort5 happy with synapse.

The two flags we set on invocation are no longer required, as they have been removed and are the default behaviour. The author has also removed dont_skip, and implies it is no longer required. I am unsure if that means we should be using something else though.

https://timothycrosley.github.io/isort/CHANGELOG/#500-penny-july-4-2020

@anoadragon453 anoadragon453 requested a review from a team July 5, 2020 14:00
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity:

  • -y meant "apply changes recursively without asking"
  • -rc meant "recursively look for Python files of which to sort imports"

These both indeed seem to be implied now given the changelog.

not_skip = '__init__.py' is now the default according to https://timothycrosley.github.io/isort/docs/configuration/profiles/#attrs, so it looks fine to remove.

tox.ini Outdated Show resolved Hide resolved
changelog.d/7786.misc Outdated Show resolved Hide resolved
Half-Shot and others added 2 commits July 5, 2020 15:44
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
@anoadragon453 anoadragon453 merged commit 62b1ce8 into develop Jul 5, 2020
@anoadragon453 anoadragon453 deleted the hs/isort-5 branch July 5, 2020 15:32

import dns.resolver
import urllib2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, is this script still Python 2?

@@ -35,6 +33,7 @@
)
from synapse.api.room_versions import KNOWN_ROOM_VERSIONS
from synapse.events import EventBase
from synapse.logging import opentracing as opentracing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from synapse.logging import opentracing as opentracing
from synapse.logging import opentracing

@@ -45,6 +43,7 @@
from synapse.module_api import ModuleApi
from synapse.push.mailer import load_jinja2_templates
from synapse.types import Requester, UserID
from synapse.util import stringutils as stringutils
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from synapse.util import stringutils as stringutils
from synapse.util import stringutils

import logging
from io import BytesIO

import PIL.Image as Image
from PIL import Image as Image
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from PIL import Image as Image
from PIL import Image

from synapse.api.errors import StoreError
from synapse.storage._base import SQLBaseStore
from synapse.types import JsonDict
from synapse.util import stringutils as stringutils
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from synapse.util import stringutils as stringutils
from synapse.util import stringutils

import mock

import signedjson.key as key
import signedjson.sign as sign
from signedjson import key as key, sign as sign
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from signedjson import key as key, sign as sign
from signedjson import key, sign

from parameterized import parameterized_class
from PIL import Image as Image
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from PIL import Image as Image
from PIL import Image

@richvdh
Copy link
Member

richvdh commented Jul 6, 2020

@auscompgeek: while those changes are all valid, they look like pre-existing problems. I don't think it makes sense to try and update this PR now it has landed.

If you (or anybody) wants to open a new PR with those changes, be my guest...

@auscompgeek
Copy link
Contributor

Whoops, sorry, I didn't realise this PR was already merged when I left those comments.

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.

4 participants