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

Add populate_all feature #26

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add populate_all feature #26

wants to merge 7 commits into from

Conversation

adhikasp
Copy link
Contributor

This add --populate-all optional argument to populate (append if exist or create it) __all__ variables with unused modules found in the code.

Closes #16

@coveralls
Copy link

coveralls commented Aug 29, 2017

Coverage Status

Coverage decreased (-0.3%) to 97.908% when pulling 1afce1b on adhikasp:all into 459f5f5 on myint:master.

@@ -112,6 +112,8 @@ Below is the full listing of options::
this only triggers if there is only one star import in
the file; this is skipped if there are any uses of
`__all__` or `del` in the file
--populate-all populate `__all__` with unused import found in the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too generic. --populate-module-dunder-all ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but... dunder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I think you mean under

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think dunder is good.

@coveralls
Copy link

coveralls commented Aug 29, 2017

Coverage Status

Coverage decreased (-0.05%) to 98.17% when pulling ad4eff0 on adhikasp:all into 459f5f5 on myint:master.

@myint
Copy link
Member

myint commented Aug 31, 2017

Thanks!

I tried this out in a file like:

from foo import bar as alpha, beta
$ ./autoflake.py --pop __init__.py
--- original/__init__.py
+++ fixed/__init__.py
@@ -1 +1,2 @@
 from foo import bar as alpha, beta
+__all__ = ['bar as alpha', 'beta']

And I got an interesting module name. 😄

@adhikasp
Copy link
Contributor Author

adhikasp commented Aug 31, 2017 via email

@myint
Copy link
Member

myint commented Sep 1, 2017

Thanks. You might also want to test imports with tabs in the code:

from alpha import beta<tab>as<tab>gamma

@coveralls
Copy link

coveralls commented Sep 4, 2017

Coverage Status

Coverage increased (+0.09%) to 98.312% when pulling 27a89d9 on adhikasp:all into 459f5f5 on myint:master.

@myint
Copy link
Member

myint commented Sep 4, 2017

Looks good. I found one other edge case. Files like the following, get an empty __all__ added to them.

def foo():
    bar = 0
--- original/foo.py
+++ fixed/foo.py
@@ -1,2 +1,3 @@
 def foo():
     bar = 0
+__all__ = []

@myint
Copy link
Member

myint commented Sep 4, 2017

I guess the other thing I notice from the above is that this new feature affects all files. I think we should be conservative and only touch __init__.py files. This follows #16 (comment).

@coveralls
Copy link

coveralls commented Oct 8, 2017

Coverage Status

Coverage increased (+0.1%) to 98.32% when pulling 310e1a3 on adhikasp:all into 459f5f5 on myint:master.

@coveralls
Copy link

coveralls commented Oct 8, 2017

Coverage Status

Coverage increased (+0.1%) to 98.32% when pulling 7b2b5ae on adhikasp:all into 459f5f5 on myint:master.

@fsouza
Copy link
Collaborator

fsouza commented Sep 18, 2022

@adhikasp do you still plan to work on this?

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

Successfully merging this pull request may close these issues.

Populate __all__ to avoid unused name errors
5 participants