Skip to content

709/from imports #760

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

Merged
merged 11 commits into from
Aug 4, 2022
Merged

Conversation

aptenodytes-forsteri
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

  • from foo import bar adds //foo as a dep instead of //foo/bar.
  • from foo.bar import baz adds //foo/bar as a dep instead of //foo/bar:baz (when //foo/bar:baz is a separate py_library that contains foo/bar/baz.py as a source).
  • from google.cloud import aiplatform does not add "@gazelle_python_test_google_cloud_aiplatform//:pkg", as a dep (fails to resolve from imports for pip packages).

Issue Number: #709

What is the new behavior?

When gazelle encounters from foo.bar import baz, try to resolve, in order from most specific to least specific:

foo.bar.baz
foo.bar
foo

This way, if foo.bar.baz is in the manifest file or in the index of existing rules (e.g. from a rule that contains the source file foo/bar/baz, it can be found). If foo.bar.baz can't be resolved, try foo.bar (maybe baz is just a function or variable in foo/bar.py).

Does this PR introduce a breaking change?

  • Yes
  • No - all existing tests continue to pass.

Other information

May want to consider antagonistic cases - are there cases where imports are truly ambiguous?

aptenodytes-forsteri and others added 5 commits July 16, 2022 23:32
- Add test case with `from __future__ import print_function`.
- Make sure that `from foo import bar, baz` works.
- Add test case for this.
- Keep error the same as before.
Copy link
Member

@f0rmiga f0rmiga left a comment

Choose a reason for hiding this comment

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

Thanks!!

@f0rmiga f0rmiga merged commit ebeb822 into bazel-contrib:main Aug 4, 2022
@aptenodytes-forsteri aptenodytes-forsteri deleted the 709/from-imports branch August 4, 2022 18:04
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.

2 participants