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

[BUG] UnexpandableImportStar without expand-stars or expand_stars=true #182

Open
Avasam opened this issue Dec 2, 2022 · 3 comments
Open
Labels
bug Something isn't working

Comments

@Avasam
Copy link
Contributor

Avasam commented Dec 2, 2022

Describe the bug A clear and concise description of what the bug is.

There's star imports of C wrapped modules in the codebase. If I tell pycln to not expand stars, it'll still fail.

To Reproduce Steps to reproduce the behavior:

  1. Take this code snippet:

     from win32api import *
  2. Run this Pycln command:

    $ pycln example.py
  3. Error traceback or unexpected output (if present):

    example.py:1:0 UnexpandableImportStar: 'win32api' module not found or it's a C wrapped module! ⛔

Expected behavior:

  1. Description: A clear and concise description of what you expected to happen.

pycln shouldn't error on unexpandable star imports if its not going to expand star imports.

Environment (please complete the following informations):

  • Python Version: Python 3.9.13
  • Pycln Version: pycln, version 2.1.1
  • OS Type: Windows 10
@Avasam Avasam added the bug Something isn't working label Dec 2, 2022
@Avasam Avasam changed the title [BUG] UnexpandableImportStar without expand-stars or expand_stars=false [BUG] UnexpandableImportStar without expand-stars or expand_stars=true Dec 2, 2022
@hadialqattan
Copy link
Owner

hadialqattan commented Dec 2, 2022

Hi,

This behavior is because without using --all flag Pycln tries to analyze star imports in order to know weather they are actually used or not (the --expand-stars flag only makes it refactor that import by keeping only the used names).

Also, you can take that error as a warning; Pycln here is trying to give you a hint that it can't conclude anything regarding that star import (it would be skipped in anyway).

Thanks.

@Avasam
Copy link
Contributor Author

Avasam commented Dec 2, 2022

Makes sense, so even if it's not going to expand, it's still analysing to see if it can remove the import. Maybe the error could be improved ? UnexpandableImportStar error lead me to think it was trying to expand and that the option was broken.

Unfortunately nothing I can do about the use of modules star imports. (not my project + outside the scope of what I'm willing to refactor + some legit module re-exports). But I do take it as a warning.

If side-effects cannot be analysed (or are analyzed as "maybe"), could pycln still remove unused explicit imports?
ie:

from c_module import used, unused
# to
from c_module import used


from c_module import unused
# to
import c_module

(I can turn this into a proper feature request if you tell me it's feasible)

For now I'll # nopycln: import those lines (and/or add the c-modules to the exclude list).

@hadialqattan
Copy link
Owner

Hi again @Avasam,

Regarding improving the UnexpandableImportStar error, I'll think of it and respond later... The second suggestion sounds good to me and I believe it should've been already implemented! So yes please open a feature request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants