feat(gazelle): Include types/stubs packages#2425
Conversation
|
Thank you @ewianda for the contribution. @dougthor42, would you have time to look at this? |
dougthor42
left a comment
There was a problem hiding this comment.
I'll probably be able to test this out sometime this week.
Until then, here are some review comments. Also, please add a flag to turn this on or off. It should be off by default for now, and then in a couple releases we can consider making it on by default.
gazelle/modules_mapping/testdata/django_types-0.15.0-py3-none-any.whl
Outdated
Show resolved
Hide resolved
dougthor42
left a comment
There was a problem hiding this comment.
Please add documentation to gazelle/README.md
Running with the default include_stub_packages = False didn't cause any problems with our large project running gazelle, so 👍 there.
Running with include_stub_packages = True didn't add any *_types or _stubs deps to targets 👎.
Is there something else I need to do? We already have some type/stub packages as part of our requirements.in (and thus requirements.txt and gazelle_python.yaml). For example, we have sqlmypy: sqlalchemy_stubs and annotated_types: annotated_types in gazelle_python.yaml
Test methodology
bazel run //:buildozer 'remove deps' '//src/pyle_xc/...:*'bazel run //:gazelle src/pyle_xcgit diffand see what has changed.
Thanks, @dougthor42, for testing. Did you fix 631ab19 Also, the logic is that |
692ff24 to
dfddffa
Compare
Ah, no I did not. I applied that change and ran diff --git a/gazelle_python.yaml b/gazelle_python.yaml
index 5752a8274a..66e8ab5c4f 100644
--- a/gazelle_python.yaml
+++ b/gazelle_python.yaml
@@ -395,6 +395,7 @@ manifest:
pymatching: PyMatching
pymodbus: pymodbus
pyparsing: pyparsing
+ pyqt5_stubs: pyqt5_stubs
pytest: pytest
pytest_asyncio: pytest_asyncio
pytest_benchmark: pytest_benchmark
@@ -484,9 +485,8 @@ manifest:
sphinxcontrib.svgbob: sphinxcontrib_svgbob
sqlalchemy: SQLAlchemy
sqlalchemy_bigquery: sqlalchemy_bigquery
- sqlmypy: sqlalchemy_stubs
+ sqlalchemy_stubs: sqlalchemy_stubs
sqlparse: sqlparse
- sqltyping: sqlalchemy_stubs
stack_data: stack_data
stim: stim
stimcirq: stimcirqSo it's is correctly pulling in new stubs (like Luckily our code doesn't import diff --git a/src/labrad/servers/GUIs/ADR/BUILD.bazel b/src/labrad/servers/GUIs/ADR/BUILD.bazel
index 0667587998..b8e716cc30 100644
--- a/src/labrad/servers/GUIs/ADR/BUILD.bazel
+++ b/src/labrad/servers/GUIs/ADR/BUILD.bazel
@@ -11,5 +11,6 @@ py_library(
"//src/pyle/datavault:util",
"@pypi//duet",
"@pypi//pyqt5",
+ "@pypi//pyqt5_stubs",
],
)
diff --git a/src/pyle/cloud/bigquery/scripts/BUILD.bazel b/src/pyle/cloud/bigquery/scripts/BUILD.bazel
index 0d10c6b62d..f86c846852 100644
--- a/src/pyle/cloud/bigquery/scripts/BUILD.bazel
+++ b/src/pyle/cloud/bigquery/scripts/BUILD.bazel
@@ -86,6 +86,7 @@ pyle_py_binary(
"@pypi//google_cloud_bigquery",
"@pypi//pandas",
"@pypi//sqlalchemy",
+ "@pypi//sqlalchemy_stubs",
],
)So overall it looks like it's WAI 👍 🎉 🎊 |
dfddffa to
556857a
Compare
These are stub files, I will be surprised if folks are importing this directly since they are meant for type-checking tools, how ever we can let |
556857a to
c9f367e
Compare
|
ping @dougthor42 is there something else you will like to change for this to go in |
|
I have merged |
dougthor42
left a comment
There was a problem hiding this comment.
Thanks for the ping. There are still two open items:
- Please add documentation to
gazelle/README.md. A comment and example here https://github.com/bazelbuild/rules_python/blob/ca987735a04c2e20e9341b4ffd24082c99afd152/gazelle/README.md?plain=1#L109-L122 is probably sufficient. - Please add a short blurb to
CHANGELOG.mddescribing the new feature.
Otherwise this LGTM.
| doc = "A set of regex patterns to match against each calculated module path. By default, exclude the modules starting with underscores.", | ||
| mandatory = False, | ||
| ), | ||
| "include_stub_packages": attr.bool( |
There was a problem hiding this comment.
Other new features get labeled as "experimental" for a couple releases (eg: experimental_requirement_cycles). Do we want to do the same here as experimental_include_stub_packages?
Given that it's opt-in and that no other gazelle features are experimental, I'm inclined to leave it as-is.
There was a problem hiding this comment.
I vote to leave it as is
c581b5d to
d1f8b9e
Compare
4e2c75b to
8a90f5b
Compare
aignas
left a comment
There was a problem hiding this comment.
Thank you @dougthor42 for the review and thank you @ewianda for the contribution. Stamping and merging.
|
|
||
| # This wheel is purely here to validate the wheel extraction code. It's not | ||
| # intended for anything else. | ||
| internal_dev_deps() |
There was a problem hiding this comment.
nit: FYI, this could return a list of repos that are created internally via the bazel skylib's extension metadata. Then bazel mod tidy would ensure that the use_repo statement in the MODULE.bazel is always up to date.
This PR adds logic that checks if a package has a corresponding
typesorstubspackage and automatically adds that to the BUILD file. This is useful for typeckers e.g pyright , mypy