Skip to content

Run Pyrefly on pandas-stubs in CI #1263

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 5 commits into from
Jun 26, 2025
Merged

Run Pyrefly on pandas-stubs in CI #1263

merged 5 commits into from
Jun 26, 2025

Conversation

yangdanny97
Copy link
Contributor

Similar to #1240, this PR adds Pyrefly to run on just the stubs directory, since there are very few false-positives now (I've filed facebook/pyrefly#569 to address some of these).

I tested the CI and added the suppressions on my fork here: yangdanny97#1

There are some known gaps that prevent us from checking the tests directory without a lot of false-positives, so that can be saved for later.

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Jun 25, 2025

One thing to consider. In CI, we have tests that separately test the stubs locally, but also test them when installed (for mypy and pyright). There have been occasions when the type checker passes the stubs locally, but fails when installed as a package. So you may want to add a test for pyrefly in that case, although maybe that doesn't make sense because you can't have pyrefly pass tests on user code (pandas-stubs/tests)

Let me know what you think about this.

@yangdanny97
Copy link
Contributor Author

yangdanny97 commented Jun 26, 2025

I was thinking that we test the installed stubs once we're able to check the tests directory. Without user code, I don't think we would emit errors for an installed dependency.

We're still some ways off from being able to do that (there are a few hundred false positives right now), so I figured we could run it on just the stubs for now and still get some value out of it.

Wdyt?

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Jun 26, 2025

I was thinking that we test the installed stubs once we're able to check the tests directory. Without user code, I don't think we would emit errors for an installed dependency.

We're still some ways off from being able to do that (there are a few hundred false positives right now), so I figured we could run it on just the stubs for now and still get some value out of it.

Wdyt?

OK - that's fine by me.

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

thanks @yangdanny97

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Jun 26, 2025

@yangdanny97 I'm going to assume that if new versions of pyrefly come out that affect these annotations, you'll update the version of pyrefly here and make any necessary changes to the stubs (e.g., you might end up removing a # pyrefly: ignore comment).

@Dr-Irv Dr-Irv merged commit 69cf85e into pandas-dev:main Jun 26, 2025
13 checks passed
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