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

Use isort v5.x as the default version #10258

Merged
merged 1 commit into from Jul 22, 2020
Merged

Conversation

asherf
Copy link
Member

@asherf asherf commented Jul 5, 2020

@asherf asherf force-pushed the py3b branch 2 times, most recently from 8a09216 to 9175808 Compare July 20, 2020 03:29
@asherf asherf changed the title Allow isort v5.x as the default version Use isort v5.x as the default version Jul 20, 2020
@coveralls
Copy link

coveralls commented Jul 20, 2020

Coverage Status

Coverage remained the same at 0.0% when pulling 217064e on asherf:py3b into 22cc05f on pantsbuild:master.

@@ -9,7 +9,7 @@ class Isort(PythonToolBase):
"""The Python import sorter tool (https://timothycrosley.github.io/isort/)."""

options_scope = "isort"
default_version = "isort>=4.3.21,<4.4"
default_version = "isort>=5.0.0,<6.0"
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Given the width of the previous range (and the fact that we don't have a lockfile here), should this be isort>=5.0.0,<5.1?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. This reduces the risk of users having something break on them if they don't end up pinning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, looks like 5.1.x is the newest. So >=5.1.4,<5.2: https://github.com/timothycrosley/isort/blob/develop/CHANGELOG.md

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems that the author is doing the right thing and not introducing breaking changes in 5.x release (when he introduced breaking changes he moved from 4.x to 5.x) hence my choice to allow all 5.x release.
Since there are changes going on there in recent weeks.

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks, Asher! I like that this current iteration changes the default to always be 5, rather than 4 or 5.

@@ -9,7 +9,7 @@ class Isort(PythonToolBase):
"""The Python import sorter tool (https://timothycrosley.github.io/isort/)."""

options_scope = "isort"
default_version = "isort>=4.3.21,<4.4"
default_version = "isort>=5.0.0,<6.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

+1. This reduces the risk of users having something break on them if they don't end up pinning.

Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Is this expected to resolve:

✓ isort succeeded.
/Users/stuhood/.pex/installed_wheels/078dc1a361398512631618298336f252e9b3a4a7/setuptools-49.2.0-py3-none-any.whl/setuptools/distutils_patch.py:26: UserWarning: Distutils was imported before Setuptools. This usage is discouraged and may exhibit undesirable behaviors or errors. Please use Setuptools' objects directly or at least import Setuptools first.
  "Distutils was imported before Setuptools. This usage is discouraged "

?

@stuhood stuhood merged commit c827416 into pantsbuild:master Jul 22, 2020
@stuhood
Copy link
Sponsor Member

stuhood commented Jul 22, 2020

(...let's find out!)

@asherf asherf deleted the py3b branch July 22, 2020 16:25
stuhood added a commit to stuhood/pants that referenced this pull request Jul 27, 2020
stuhood added a commit that referenced this pull request Jul 27, 2020
This reverts commit c827416.

Between isort 4 and isort 5, the requirement on which files are present in the sandbox has changed, and this can result in unstable sort orders. See PyCQA/isort#1147 for the likely cause. Under isort 5, the following set of roots (via `./pants --changed-parent=master list` can result in an order that does not align with `./pants list ::`: https://gist.github.com/stuhood/ec04e86b8a1f44d8b11dcb79a1732a8f

In order to use isort 5 stably in a sandbox, we will need to include the necessary transitive dependencies for it to execute its new first-party-detection algorithm, or disable it.

[ci skip-rust-tests]
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.

None yet

4 participants