Do this because there are scenarios where zipped up packages end up being imported, and the import classification logic was marking those as part of the standard library.

Trying to do full TDD. So opening with just a commit to add tests to hopefully get a nice CI failure first.

Resolves #46

It seems I need this for the issue I opened yesterday on reorder_python_imports regarding multiple imports on one line.

Let me know what you think, I guess it's pretty aggressive though :)

I removed the restriction on ImportImport as well as it is possible and legal to do (Although pep8 disapproves). I seem to have read somewhere that you should follow the pep but it's okay not to, or something like that, somehwere, can't for the life of me find now though

I am using this, and I noticed the relative import sorted incorrect. Here is the code

import pprint
from aspy.refactor_imports.import_obj import FromImport

objs = sorted([
    FromImport.from_str('from a import foo'),
    FromImport.from_str('from a.b import baz'),
    FromImport.from_str('from a import bar'),
    FromImport.from_str('from a import bar as buz'),
    FromImport.from_str('from a import bar as baz'),
    FromImport.from_str('from .a import bar as baz'),
    FromImport.from_str('from . import bar as baz'),
    FromImport.from_str('from .aa.bb import bar as baz'),
])
pprint.pprint([i.to_text() for i in objs])

Excepted:

[u'from a import bar\n',
 u'from a import bar as baz\n',
 u'from a import bar as buz\n',
 u'from a import foo\n',
 u'from a.b import baz\n',
 u'from .aa.bb import bar as baz\n',
 u'from .a import bar as baz\n',
 u'from . import bar as baz\n',
]

Got:

[u'from . import bar as baz\n',
 u'from .a import bar as baz\n',
 u'from .aa.bb import bar as baz\n',
 u'from a import bar\n',
 u'from a import bar as baz\n',
 u'from a import bar as buz\n',
 u'from a import foo\n',
 u'from a.b import baz\n']

In Python 3.10+, we have access to sys.stdlib_module_names, which lets us know all of the modules that are considered part of the Python stdlib — regardless of whether they are compiled into the current Python interpreter.

In earlier versions of Python 3, we have only sys.builtin_module_names, which tells us the modules that are compiled into the current interpreter.

To work around this, we need a solution to tell us whether a module that is not compiled into the current interpreter should be considered part of the stdlib. The previous approach considered all paths inside sys.path, excluding those ending with -packages (to exclude site-packages and dist-packages), to contain only stdlib modules. We also excluded the paths found in the PYTHONPATH environment variable, reasoning that if a path was present in sys.path because it was added by PYTHONPATH, it was a path falling outside the default Python interpreter paths. If we found the module in question within one of these paths, we'd report it as a builtin.

With this change, we instead use distutils.sysconfig.get_python_lib(standard_lib=True), which directly reports the path used for the standard library (and not third-party extensions).

Finally, we're making the order of resolution consistent among Python versions. We check, in priority order:

1. If the module is specified in unclassifiable_application_modules (this is our "manual override")
2. If the module is in the list of known builtins
3. If the module can be found in the application path, influenced by the application_directories setting
4. If the module can be found in the standard library path If none of the above match, we assume the import is a third-party module.

Under Python 3.10+, we expect that check 4 never matches without check 2 having matched first.

All of this also allows us to reduce much of the code duplication introduced in bdc924875711a6f7a9239cec92f9cafe4c3ba657.

Tests are updated to reflect these new cases, and tox.ini adds all the interpreter versions we care about. We're also able to remove a previous version-specific test skip that was necessary due to the version-inconsistent behavior, but no longer performs differently among versions.

Fixes #155.

Alternative to https://github.com/asottile/aspy.refactor_imports/pull/106 , reworking classify_import to take an import object. This is backwards incompatible, there are a few possible uses out there: https://github.com/search?l=Python&q=classify_import&type=Code .

Fixes asottile/reorder_python_imports#214 .

This fixes a bug on Windows, where under certain circumstances an application is misclassified as a builtin, due to case-sensitive path comparison on a case-insensitive filesystem. See #52 for more details.

Closes #52

Proposed fixes

• Compute the common path prefix of the module path and the application directory, using os.path.commonpath. Check if this path prefix is the same directory as the application directory, using ~os.path.samefile~ os.path.normcase.

• Handle the case where the module path is on a different drive than the application directory. The os.path.commonpath function raises ValueError if the paths are on different drives.

• Use ~os.path.samefile~ os.path.normcase to check if the real and absolute paths of the module are the same directory.

Tests

The test case uses an application directory named SRC and a sys.path entry of src. This is slightly different from the real world case described in the issue, but suffices to reproduce the issue. The test case is marked xfail on Linux, because this platform typically uses case-sensitive file systems.

Surprisingly (at least to me), the test case also needs to be marked xfail on macOS, even though this platform is traditionally case-insensitive. The reason for this is that os.path.commonpath is case-sensitive on macOS (arguably a bug in CPython?).

There is no test case yet for the branch where module path and application directory are on different drives, because I have not come up with a good way to test this.

Since dataclasses doesn't exist before 3.7 so I use a backport library of it https://github.com/ericvsmith/dataclasses to support from python 3.6 to python 3.8. However, classify_import on dataclasses on python 3.6 returns different import ImportType (THIRD_PARTY) from the value that is returned by 3.7 (BUILTIN), so the sort order is different between python versions. I thought that the backport option can pin them as BUILTINs and fix the situation.

unclassifiable_application_modules

Under Python 3.10+, the system builtins are checked for the import before anything else.

Under Python <3.10, unclassifiable_application_modules is checked before anything else.

As a result, if a builtin module is specified in unclassifiable_application_modules, it will be reported as an application module under Python <3.10 and a builtin under Python 3.10+.

I'm not sure what the "correct" behavior for these are, but it does seem that both should be consistent among Python versions.

application_directories

Furthermore, the difference in find_stdlib under Python <3.10 is different from the base in sys.stdlib_module_names check under 3.10+. If the former is configured with an empty set of application_directories, it will identify an arbitrary module in the current directory as a builtin, while sys.stdlib_module_names does not. Here the behavior seems more clearly correct under Python 3.10+.

I'm using python3.8.

from aspy.refactor_imports.classify import classify_import
print(classify_import("distutils"))

Expected: BUILTIN

Actual: THIRD_PARTY

Upon a closer look, I see that you specifically force distutils to be classified as THIRD_PARTY though I don't understand the reasoning given it still seems to be part of the standard library or at least seems to be documented as such. Also, I don't see functionality such as distutils.dir_util.copy_tree having moved into setuptools.

    # force distutils to be "third party" after being gobbled by setuptools
    elif base == 'distutils':
        return ImportType.THIRD_PARTY

I get the following assertion error while running tox -e linting on the pytest repo locally, which runs pre-commit, which runs reorder-python-imports:

Reorder python imports...................................................Failed
- hook id: reorder-python-imports
- exit code: 1

Traceback (most recent call last):
  File "/home/ran/.cache/pre-commit/repoye77qfmi/py_env-python3.10/bin/reorder-python-imports", line 8, in <module>
    sys.exit(main())
  File "/home/ran/.cache/pre-commit/repoye77qfmi/py_env-python3.10/lib/python3.10/site-packages/reorder_python_imports.py", line 907, in main
    retv |= _fix_file(filename, args)
  File "/home/ran/.cache/pre-commit/repoye77qfmi/py_env-python3.10/lib/python3.10/site-packages/reorder_python_imports.py", line 475, in _fix_file
    new_contents = fix_file_contents(
  File "/home/ran/.cache/pre-commit/repoye77qfmi/py_env-python3.10/lib/python3.10/site-packages/reorder_python_imports.py", line 456, in fix_file_contents
    partitioned = step(partitioned)
  File "/home/ran/.cache/pre-commit/repoye77qfmi/py_env-python3.10/lib/python3.10/site-packages/reorder_python_imports.py", line 366, in apply_import_sorting
    sorted_blocks = sort(import_obj_to_partition.keys(), **sort_kwargs)
  File "/home/ran/.cache/pre-commit/repoye77qfmi/py_env-python3.10/lib/python3.10/site-packages/aspy/refactor_imports/sort.py", line 98, in sort
    imports_partitioned[classify_func(import_obj)].append(import_obj)
  File "/home/ran/.cache/pre-commit/repoye77qfmi/py_env-python3.10/lib/python3.10/site-packages/aspy/refactor_imports/sort.py", line 72, in classify_func
    tp = classify_import(obj.import_statement.module, **kwargs)
  File "/home/ran/.cache/pre-commit/repoye77qfmi/py_env-python3.10/lib/python3.10/site-packages/aspy/refactor_imports/classify.py", line 135, in classify_import
    found, module_path, is_builtin = _get_module_info(
  File "/home/ran/.cache/pre-commit/repoye77qfmi/py_env-python3.10/lib/python3.10/site-packages/aspy/refactor_imports/classify.py", line 103, in _get_module_info
    assert spec.submodule_search_locations is not None
AssertionError

reorder_python_imports is configured to run like this:

-   repo: https://github.com/asottile/reorder_python_imports  
    rev: v2.6.0
    hooks:
    -   id: reorder-python-imports
        args: ['--application-directories=.:src', --py37-plus]

This is on Python 3.10. I will try to add more details later if needed.

There is a special case logic gate in classify.py:

# special case pypy3 bug(?)
elif not os.path.exists(spec.origin):
    return True, '(builtin)', True

If you are using a zipped third-party package, then this will falsely identify it as part of the standard library.

Here's a code snippet showing the value of spec.origin in such a case:

Python 3.7.5 (default, Oct 31 2019, 15:18:51) [MSC v.1916 64 bit (AMD64)]
Type 'copyright', 'credits' or 'license' for more information
IPython 7.9.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import importlib.util

In [2]: spec = importlib.util.find_spec('py4j')

In [3]: spec
Out[3]: ModuleSpec(name='py4j', loader=<zipimporter object "C:\DevApps\spark\spark-2.4.4-bin\python\lib\py4j-0.10.7-src.zip">, origin='C:\\DevApps\\spark\\spark-2.4.4-bin\\python\\lib\\py4j-0.10.7-src.zip\\py4j\\__init__.py', submodule_search_locations=['C:\\DevApps\\spark\\spark-2.4.4-bin\\python\\lib\\py4j-0.10.7-src.zip\\py4j'])

In [4]: spec.origin
Out[4]: 'C:\\DevApps\\spark\\spark-2.4.4-bin\\python\\lib\\py4j-0.10.7-src.zip\\py4j\\__init__.py'

That spec.origin doesn't exist on the filesystem, so os.path.exists(spec.origin) is False.

I realize we're using %PythonPath% which isn't really supported, but I do believe this would also be an issue in other scenarios. (Sourcing py4j from a zip included with a Spark distribution is actually a fairly common example I believe.)

Would it be reasonable to get another logic gate in there to identify the zipped library scenario? I'd be happy to try and make the addition if you like.

Thanks for the great tool. We're fine with mixing our third-party and first-party imports due to using %PythonPath%. We really appreciate the black compatibility (and the focus on clean diffs).