-
Notifications
You must be signed in to change notification settings - Fork 196
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
Support relative imports in AddImportsVisitor. #585
Conversation
* Adds an Import dataclass to represent a single imported object * Refactors AddImportsVisitor to pass around Import objects * Separates out the main logic in get_absolute_module_for_import so that it can be used to resolve relative module names outside of a cst.Import node * Resolves relative module names in AddImportsVisitor if we have a current module name set. Fixes #578
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall nice!
How about a test case with a from .. import E as foo
style import?
Codecov Report
@@ Coverage Diff @@
## main #585 +/- ##
==========================================
- Coverage 94.73% 94.72% -0.01%
==========================================
Files 241 242 +1
Lines 24660 24772 +112
==========================================
+ Hits 23362 23466 +104
- Misses 1298 1306 +8
Continue to review full report at Codecov.
|
libcst/codemod/visitors/_imports.py
Outdated
mod = self | ||
# `import ..a` -> `from .. import a` | ||
if mod.relative and mod.obj is None: | ||
mod = replace(mod, module_name="", obj=mod.module_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately replace
's first argument is also called obj
, so this won't work (as shown by the unit tests)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh. any idea why the local tests passed? (perhaps because i only ran them on 3.9?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this only seems to be a problem with 3.6 because we use the dataclasses package from pypi, where obj
is not a positional-only argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See also https://bugs.python.org/issue37163
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks! I'll merge if CI is green.
it can be used to resolve relative module names outside of a cst.Import
node
current module name set.
Fixes #578