public symbol filtering with pyrefly report --public-only #3104#3162
public symbol filtering with pyrefly report --public-only #3104#3162jorenham wants to merge 5 commits into
pyrefly report --public-only #3104#3162Conversation
This comment has been minimized.
This comment has been minimized.
|
Hey @stroxler, are you familiar with Pyrefly report? Would you be able to take a look at this? |
1 similar comment
|
Hey @stroxler, are you familiar with Pyrefly report? Would you be able to take a look at this? |
This comment has been minimized.
This comment has been minimized.
|
@migeed-z has imported this pull request. If you are a Meta employee, you can view this in D101673000. |
connernilsen
left a comment
connernilsen
left a comment
There was a problem hiding this comment.
Hey @jorenham, thanks for doing this! I have a few recommendations, but can you rebase and re-request review once you're ready again?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
grievejia
left a comment
grievejia
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
|
@grievejia sorry but I can't see your review |
|
also adding @grievejia's remarks to the review |
…ering public symbols
…rt` public symbol filtering Co-authored-by: Jia Chen <grievejia@meta.com>
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
grievejia
left a comment
grievejia
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
4 participants
Summary
In #3104 I proposed that
pyrefly reportalso includes an export graph so we'd be able to figure out intypestatswhat the public symbols are as post-processing step.With this change, that is no longer needed. Because now,
pyrefly report --public-onlywill report only the public symbols, using the cross-module tracing algorithm from typestats.Right now this is opt-in. But we could also make this the default, and instead add a
--include-privateflag or something. Thoughts?Fixes #3104
Test Plan
Tests added