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

Split basic import statements with multiple imported names #141

Merged
merged 4 commits into from
Mar 16, 2022

Conversation

amyreese
Copy link
Member

@amyreese amyreese commented Mar 4, 2022

Before sorting and merging imports within a block, this looks for basic
(non-from) imports with more than one imported name, and then splits
that import into a separate import for each name. Comments are
duplicated from the original import to all split imports.

Fixes #140

Docs preview: https://usort--141.org.readthedocs.build/en/141/

@amyreese amyreese added the enhancement New feature or request label Mar 4, 2022
@amyreese amyreese added this to the 1.1 milestone Mar 4, 2022
@amyreese amyreese requested a review from thatch March 4, 2022 02:19
@amyreese amyreese self-assigned this Mar 4, 2022
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 4, 2022
@amyreese
Copy link
Member Author

amyreese commented Mar 4, 2022

  • Still needs documentation.

@amyreese
Copy link
Member Author

amyreese commented Mar 5, 2022

Some notes from discussion:

  • Consider if old behavior is potentially unstable in 1.0.1, (eg, import thirdparty, os gets changed to import os, thirdparty). If unstable, submit separate bug fix for a 1.0.2 release.
  • Look for test cases that might fail if evolve() isn't used.
  • Add test case covering combined split+merge with comments.
  • Determine if new behavior is fully "back compatible", ie, will 1.0 or 0.6.4 preserve changes made when splitting.
  • Consider if this warrants a 2.0 version bump

Before sorting and merging imports within a block, this looks for basic
(non-from) imports with more than one imported name, and then splits
that import into a separate import for each name. Comments are
duplicated from the original import to all split imports.

Fixes facebook#140
@amyreese
Copy link
Member Author

amyreese commented Mar 8, 2022

The existing test case already fails if evolve() is not used:

AssertionError: µsort result did not match expected value:

Before:
-------

import sys, os, fancy, time, re
import math, usort, bisect

Expected:
---------

import bisect
import math
import os
import re
import sys
import time

import fancy
import usort

Result:
-------

import bisect
import math

import os

import re

import sys

import time

import fancy
import usort

Because fancy was part of the same group, but a different category, the blank line meant to separate the categories ends up applying to the stdlib imports too.

@amyreese amyreese merged commit 63839ac into facebook:main Mar 16, 2022
@amyreese amyreese deleted the split branch March 16, 2022 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: split basic imports
2 participants