Ensure py.typed is present in all packages (BUILD-660) #27491
Merged
+95
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
smackesey
requested review from
alangenfeld and
schrockn
and removed request for
neverett
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Comment on lines
+48
to
+53
| def git_ls_files(pattern: str) -> list[str]: | ||
| return ( | ||
| subprocess.run(["git", "ls-files", pattern], check=True, text=True, capture_output=True) | ||
| .stdout.strip() | ||
| .split("\n") | ||
| ) |
There was a problem hiding this comment.
When git ls-files finds no matches, stdout.strip() returns an empty string, which split('\n') converts to ['']. To handle this case correctly, the code should return an empty list instead:
.split('\n') if stdout.strip() else []Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
smackesey
force-pushed
the
sean/py-typed-packages
branch
from
8e0d228 to
65ff6fc
Compare
|
Deploy preview for dagster-docs ready! Preview available at https://dagster-docs-crvhf1x5x-elementl.vercel.app Direct link to changed pages: |
smackesey
force-pushed
the
sean/py-typed-packages
branch
from
65ff6fc to
6ae2e37
Compare
smackesey
force-pushed
the
graphite-base/27491
branch
from
357f918 to
79a0cdb
Compare
smackesey
changed the base branch from
graphite-base/27491
to
sean/components/components-py-typed
This was referenced Feb 2, 2025
Base automatically changed from
sean/components/components-py-typed
to
master
February 2, 2025 21:00
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary & Motivation
py.typedto all Python packages (anything with a setup.py that can be installed into an environment)py.typed, and all published packages include it in theirMANIFEST.inThere are a few reasons for this:
py.typedis not present. In particular, pyright/pylance will not use annotations from a non-py.typedpackage unlessuseLibraryCodeForTypesis enabled (and we don't control whether our users enable this). Not all of our packages are 100% typed, but they at least do not provide wrong type information (which some packages, especially those using fancy metaprogramming, do), which means it is safe to expose the type information they do have.py.typedto avoid some strange behavior aroundmake quick_pyright, which runs in our configured pre-push hooks. If only some files in a non-py.typedpackage are touched in a diff, it can lead to odd import resolution errors. This occurs because the files not touched by the diff are both not passed directly topyrightfor inspection, and also unavailable from the venv environmentpyrightuses due to lackingpy.typed. The result is the occasional seemingly random error whenmake quick_pyrightis run-- this PR fixes that.How I Tested These Changes
See notes on above test.
Changelog
All published dagster libraries now include a
py.typedfile, which means their type annotations will be used by static analyzers. Previously a few libraries were missing this file.