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

Import.statement produces invalid code #1641

Closed
Chilipp opened this issue Jan 22, 2021 · 3 comments
Closed

Import.statement produces invalid code #1641

Chilipp opened this issue Jan 22, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@Chilipp
Copy link

Chilipp commented Jan 22, 2021

Hey @timothycrosley! Thanks a lot for this awesome project! I was just wondering about the new (and useful) function you added with version 5.7.0: find_imports_in_code (and the corresponding _stream equivalents.

Consider the following example:

from isort.api import find_imports_in_code
code = """
from typing import List
"""
imports = list(find_imports_in_code(code))
imports[0].statement()
# Output: 'import typing.List'

this (import typing.List) is invalid code. Shouldn't imports[0].statement() rather produce from typing import List?

I'd replace the statement method

isort/isort/identify.py

Lines 25 to 31 in 6230dc3

def statement(self) -> str:
full_path = self.module
if self.attribute:
full_path += f".{self.attribute}"
if self.alias:
full_path += f" as {self.alias}"
return f"{'cimport' if self.cimport else 'import'} {full_path}"

with something like

def statement(self) -> str: 
    import_cmd = 'cimport' if self.cimport else 'import'
    if self.attribute:
        import_string = f"from {self.module} {import_cmd} {self.attribute}"
    else:
        import_string = f"{import_cmd} {self.module}"
    if self.alias: 
        import_string += f" as {self.alias}" 
    return import_string

what do you think?

@timothycrosley timothycrosley added the enhancement New feature or request label Jan 31, 2021
@timothycrosley
Copy link
Member

Hi @Chilipp,

Glad you find the project useful! I'm on board to improve the statement method (though with the disclaimer it may not print out the optimal statement - just a functional one going forward). My personal use for the new functionality was just to quickly identify imports for use in making project dependency graphs faster than can be done with full AST parsing as is generally used (for things like automatic Bazel integration) - there it's not at all important that statement results in functioning code - so I hadn't even considered it! Glad to hear others find the functionality useful!

@timothycrosley timothycrosley self-assigned this Feb 8, 2021
@timothycrosley
Copy link
Member

This is now fixed in develop and will make its way out in the next release

@Chilipp
Copy link
Author

Chilipp commented Feb 8, 2021

great! thanks a lot @timothycrosley for the quick fix 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants