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

Feature request: Ship isort 5 #12932

Closed
bersbersbers opened this issue Jul 13, 2020 · 10 comments
Closed

Feature request: Ship isort 5 #12932

bersbersbers opened this issue Jul 13, 2020 · 10 comments
Assignees
Labels
area-formatting feature-request Request for new features or functionality

Comments

@bersbersbers
Copy link

I was trying out

"python.sortImports.args": [
        "--profile",
        "black",
    ],

today, which promises better compatibility between isort and black. However, it seems that the extension ships with an older version of isort that does not support these flags.

user@host:~/.vscode-server/extensions/ms-python.python-2020.6.91350/pythonFiles/lib/python/isort> grep version __init__.py
__version__ = "4.3.21"

Even worse, adding these flags leads to endless "Saving xyz.py: Applying code action 'Sort imports'."

Shipping isort>=5 should solve this.

@bersbersbers bersbersbers added triage-needed Needs assignment to the proper sub-team bug Issue identified by VS Code Team member as probable bug labels Jul 13, 2020
@karthiknadig
Copy link
Member

@bersbersbers isort shipped 5.* in July. Looks like they have been rapidly releasing new versions almost everyday. Which tells me this is not stable yet. We will update to latest isort once we see that it is stable.

@karthiknadig karthiknadig added feature-request Request for new features or functionality needs decision and removed bug Issue identified by VS Code Team member as probable bug labels Jul 13, 2020
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Jul 14, 2020
@janosh
Copy link

janosh commented Jul 17, 2020

I've been using isort 5 since it's release date and had no issues at all. In fact, it's been a major improvement in all respects! Really looking forward to it shipping with the Python extension!

@karrtikr karrtikr mentioned this issue Jul 30, 2020
10 tasks
@karrtikr
Copy link

karrtikr commented Jul 31, 2020

Unexpected changes with upgrading isort:

Changes we'll have to make:

  • Remove old workaround used with isort 4:
    In sortImports.py, remove the following change: fc34f9c
    isort5 expects stdin stream to be of type string, not byte. And the workaround converts stream to bytes, so we have an error when upgrading. From my understanding, the workaround was added because of No output when stdin used on Windows PyCQA/isort#1195 with isort4, but isort5 doesn't have that issue, so we can remove it. cc @PeterJCLaw

  • Change tests which accept old arguments:
    Some tests in extension.sort.test.ts have specified -sp in args which will need to be changed to --sp given the new usage.

  • Change tests which expect old results:
    Some tests in extension.sort.test.ts expects old results, which are now outdated. Change the tests to compare against the new value.

  • Do something to migrate user arguments in python.sortImports.args setting & the config file ???:
    Users maybe using certain flags like -sp which needs to be updated to --sp, else their setting would break. It's probably worth it to point users to the upgrade guide.
    UPDATE: Show a general warning prompt pointing to the upgrade guide, in case we see any warnings related to deprecation from isort.

@PeterJCLaw
Copy link

PeterJCLaw commented Jul 31, 2020

  • Remove old workaround used with isort 4:
    In sortImports.py, remove the following change: fc34f9c
    isort5 expects stdin stream to be of type string, not byte. And the workaround converts stream to bytes, so we have an error when upgrading. From my understanding, the workaround was added because of timothycrosley/isort#1195 with isort4, but isort5 doesn't have that issue, so we can remove it. cc @PeterJCLaw

Yup, that's correct regarding the workaround. If isort>=5 has fixed the Windows stdin issue on its own then that workaround can be dropped. (I don't tend to develop on Windows, so haven't tested that).
It was the vscode-python CI which originally found the issue, so if the CI is happy everything should be fine.

@bersbersbers
Copy link
Author

Since this bug is receiving attention from #12949, I should note that the endless "Sorting imports" may have been that exact bug (#12949), namely, using a too recent version of setuptools. I can further test this if required.

@karrtikr karrtikr mentioned this issue Aug 7, 2020
10 tasks
@karrtikr
Copy link

Next release will have the new isort

@PeterJCLaw
Copy link

Apologies if this isn't the right place to raise this, but as I understand it, isort 5 drops support for Python 3.5 (at least being run by Python 3.5, I don't know if it can still operate on Python 3.5 sources). Does shipping isort 5 mean that the Python extension is now also dropping support for Python 3.5 generally in the next release?
Since Python 3.5 is still the default in a variety of older but still supported Linuxes (for example: anything based on Debian Stretch, which is itself expected to continue with extended support until 2022), this feels like it would be worth clarifying.

@timothycrosley
Copy link

@PeterJCLaw

at least being run by Python 3.5, I don't know if it can still operate on Python 3.5 sources

isort is very lenient about the source target since it doesn't have to parse the complete Python source code, just import statements, a subset of code that syntactically changes much less often. While it now needs Python 3.6+ to run, it can still be run against code that targets Python 2.6+ officially. Unofficially, it works just fine even against most code snippets targeting Python 1.6.1.

@karrtikr
Copy link

karrtikr commented Aug 22, 2020

@PeterJCLaw We have clarified it further in #13459 for you

@timothycrosley Thanks, but the point is that users can't select python interpreter less than 3.6 and run sorting now, right? So it may reflect on us dropping support for Python 2 and Python less than 3.6

Can users unofficially use interpreter less than Python3.6 to run isort?

@timothycrosley
Copy link

Can users unofficially use interpreter less than Python3.6 to run isort?

@karrtikr no they cannot. I was only answering the secondary question of if it's possible to run isort to sort imports within source code that targets an older (or different) Python version than isort itself is running on.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-formatting feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

6 participants